diff --git a/pyproject.toml b/pyproject.toml index 4f87c91d..e485ef61 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -146,6 +146,3 @@ omit = [ "*/telemetry.py", # External analytics; tested lightly, excluded from strict coverage target "*/services/migration_service.py", # Complex migration scenarios ] - -[tool.logfire] -ignore_no_config = true diff --git a/specs/SPEC-1 Specification-Driven Development Process.md b/specs/SPEC-1 Specification-Driven Development Process.md deleted file mode 100644 index de4053b7..00000000 --- a/specs/SPEC-1 Specification-Driven Development Process.md +++ /dev/null @@ -1,156 +0,0 @@ ---- -title: 'SPEC-1: Specification-Driven Development Process' -type: spec -permalink: specs/spec-1-specification-driven-development-process -tags: -- process -- specification -- development -- meta ---- - -# SPEC-1: Specification-Driven Development Process - -## Why -We're implementing specification-driven development to solve the complexity and circular refactoring issues in our web development process. -Instead of getting lost in framework details and type gymnastics, we start with clear specifications that drive implementation. - -The default approach of adhoc development with AI agents tends to result in: -- Circular refactoring cycles -- Fighting framework complexity -- Lost context between sessions -- Unclear requirements and scope - -## What -This spec defines our process for using basic-memory as the specification engine to build basic-memory-cloud. -We're creating a recursive development pattern where basic-memory manages the specs that drive the development of basic-memory-cloud. - -**Affected Areas:** -- All future component development -- Architecture decisions -- Agent collaboration workflows -- Knowledge management and context preservation - -## How (High Level) - -### Specification Structure - -Name: Spec names should be numbered sequentially, followed by a description eg. `SPEC-X - Simple Description.md`. -See: [[Spec-2: Slash Commands Reference]] - -Every spec is a complete thought containing: -- **Why**: The reasoning and problem being solved -- **What**: What is affected or changed -- **How**: High-level approach to implementation -- **How to Evaluate**: Testing/validation procedure -- Additional context as needed - -### Living Specification Format - -Specifications are **living documents** that evolve throughout implementation: - -**Progress Tracking:** -- **Completed items**: Use ✅ checkmark emoji for implemented features -- **Pending items**: Use `- [ ]` GitHub-style checkboxes for remaining tasks -- **In-progress items**: Use `- [x]` when work is actively underway - -**Status Philosophy:** -- **Avoid static status headers** like "COMPLETE" or "IN PROGRESS" that become stale -- **Use checklists within content** to show granular implementation progress -- **Keep specs informative** while providing clear progress visibility -- **Update continuously** as understanding and implementation evolve - -**Example Format:** -```markdown -### ComponentName -- ✅ Basic functionality implemented -- ✅ Props and events defined -- - [ ] Add sorting controls -- - [ ] Improve accessibility -- - [x] Currently implementing responsive design -``` - -This creates **git-friendly progress tracking** where `[ ]` easily becomes `[x]` or ✅ when completed, and specs remain valuable throughout the development lifecycle. - - -## Claude Code - -We will leverage Claude Code capabilities to make the process semi-automated. - -- Slash commands: define repeatable steps in the process (create spec, implement, review, etc) -- Agents: define roles to carry out instructions (front end developer, baskend developer, etc) -- MCP tools: enable agents to implement specs via actions (write code, test, etc) - -### Workflow -1. **Create**: Write spec as complete thought in `/specs` folder -2. **Discuss**: Iterate and refine through agent collaboration -3. **Implement**: Hand spec to appropriate specialist agent -4. **Validate**: Review implementation against spec criteria -5. **Document**: Update spec with learnings and decisions - -### Slash Commands - -Claude slash commands are used to manage the flow. -These are simple instructions to help make the process uniform. -They can be updated and refined as needed. - -- `/spec create [name]` - Create new specification -- `/spec status` - Show current spec states -- `/spec implement [name]` - Hand to appropriate agent -- `/spec review [name]` - Validate implementation - -### Agent Orchestration - -Agents are defined with clear roles, for instance: - -- **system-architect**: Creates high-level specs, ADRs, architectural decisions -- **vue-developer**: Component specs, UI patterns, frontend architecture -- **python-developer**: Implementation specs, technical details, backend logic -- -- Each agent reads/updates specs through basic-memory tools. - -## How to Evaluate - -### Success Criteria -- Specs provide clear, actionable guidance for implementation -- Reduced circular refactoring and scope creep -- Persistent context across development sessions -- Clean separation between "what/why" and implementation details -- Specs record a history of what happened and why for historical context - -### Testing Procedure -1. Create a spec for an existing problematic component -2. Have an agent implement following only the spec -3. Compare result quality and development speed vs. ad-hoc approach -4. Measure context preservation across sessions -5. Evaluate spec clarity and completeness - -### Metrics -- Time from spec to working implementation -- Number of refactoring cycles required -- Agent understanding of requirements -- Spec reusability for similar components - -## Notes -- Start simple: specs are just complete thoughts, not heavy processes -- Use basic-memory's knowledge graph to link specs, decisions, components -- Let the process evolve naturally based on what works -- Focus on solving the actual problem: Manage complexity in development - -## Observations - -- [problem] Web development without clear goals and documentation circular refactoring cycles #complexity -- [solution] Specification-driven development reduces scope creep and context loss #process-improvement -- [pattern] basic-memory as specification engine creates recursive development loop #meta-development -- [workflow] Five-step process: Create → Discuss → Implement → Validate → Document #methodology -- [tool] Slash commands provide uniform process automation #automation -- [agent-pattern] Three specialized agents handle different implementation domains #specialization -- [success-metric] Time from spec to working implementation measures process efficiency #measurement -- [learning] Process should evolve naturally based on what works in practice #adaptation -- [format] Living specifications use checklists for progress tracking instead of static status headers #documentation -- [evolution] Specs evolve throughout implementation maintaining value as working documents #continuous-improvement - -## Relations - -- spec [[Spec-2: Slash Commands Reference]] -- spec [[Spec-3: Agent Definitions]] diff --git a/specs/SPEC-10 Unified Deployment Workflow and Event Tracking.md b/specs/SPEC-10 Unified Deployment Workflow and Event Tracking.md deleted file mode 100644 index 91ec8860..00000000 --- a/specs/SPEC-10 Unified Deployment Workflow and Event Tracking.md +++ /dev/null @@ -1,569 +0,0 @@ ---- -title: 'SPEC-10: Unified Deployment Workflow and Event Tracking' -type: spec -permalink: specs/spec-10-unified-deployment-workflow-event-tracking -tags: -- workflow -- deployment -- event-sourcing -- architecture -- simplification ---- - -# SPEC-10: Unified Deployment Workflow and Event Tracking - -## Why - -We replaced a complex multi-workflow system with DBOS orchestration that was proving to be more trouble than it was worth. The previous architecture had four separate workflows (`tenant_provisioning`, `tenant_update`, `tenant_deployment`, `tenant_undeploy`) with overlapping logic, complex state management, and fragmented event tracking. DBOS added unnecessary complexity without providing sufficient value, leading to harder debugging and maintenance. - -**Problems Solved:** -- **Framework Complexity**: DBOS configuration overhead and fighting framework limitations -- **Code Duplication**: Multiple workflows implementing similar operations with duplicate logic -- **Poor Observability**: Fragmented event tracking across workflow boundaries -- **Maintenance Overhead**: Complex orchestration for fundamentally simple operations -- **Debugging Difficulty**: Framework abstractions hiding simple Python stack traces - -## What - -This spec documents the architectural simplification that consolidates tenant lifecycle management into a unified system with comprehensive event tracking. - -**Affected Areas:** -- Tenant deployment workflows (provisioning, updates, undeploying) -- Event sourcing and workflow tracking infrastructure -- API endpoints for tenant operations -- Database schema for workflow and event correlation -- Integration testing for tenant lifecycle operations - -**Key Changes:** -- **Removed DBOS entirely** - eliminated framework dependency and complexity -- **Consolidated 4 workflows → 2 unified deployment workflows (deploy/undeploy)** -- **Added workflow tracking system** with complete event correlation -- **Simplified API surface** - single `/deploy` endpoint handles all scenarios -- **Enhanced observability** through event sourcing with workflow grouping - -## How (High Level) - -### Architectural Philosophy -**Embrace simplicity over framework complexity** - use well-structured Python with proper database design instead of complex orchestration frameworks. - -### Core Components - -#### 1. Unified Deployment Workflow -```python -class TenantDeploymentWorkflow: - async def deploy_tenant_workflow(self, tenant_id: str, workflow_id: UUID, image_tag: str = None): - # Single workflow handles both initial provisioning AND updates - # Each step is idempotent and handles its own error recovery - # Database transactions provide the durability we need - await self.start_deployment_step(workflow_id, tenant_uuid, image_tag) - await self.create_fly_app_step(workflow_id, tenant_uuid) - await self.create_bucket_step(workflow_id, tenant_uuid) - await self.deploy_machine_step(workflow_id, tenant_uuid, image_tag) - await self.complete_deployment_step(workflow_id, tenant_uuid, image_tag, deployment_time) -``` - -**Key Benefits:** -- **Handles both provisioning and updates** in single workflow -- **Idempotent operations** - safe to retry any step -- **Clean error handling** via simple Python exceptions -- **Resumable** - can restart from any failed step - -#### 2. Workflow Tracking System - -**Database Schema:** -```sql -CREATE TABLE workflow ( - id UUID PRIMARY KEY, - workflow_type VARCHAR(50) NOT NULL, -- 'tenant_deployment', 'tenant_undeploy' - tenant_id UUID REFERENCES tenant(id), - status VARCHAR(20) DEFAULT 'running', -- 'running', 'completed', 'failed' - workflow_metadata JSONB DEFAULT '{}' -- image_tag, etc. -); - -ALTER TABLE event ADD COLUMN workflow_id UUID REFERENCES workflow(id); -``` - -**Event Correlation:** -- Every workflow operation generates events tagged with `workflow_id` -- Complete audit trail from workflow start to completion -- Events grouped by workflow for easy reconstruction of operations - -#### 3. Parameter Standardization -All workflow methods follow consistent signature pattern: -```python -async def method_name(self, session: AsyncSession, workflow_id: UUID | None, tenant_id: UUID, ...) -``` - -**Benefits:** -- **Consistent event tagging** - all events properly correlated -- **Clear method contracts** - workflow_id always first parameter -- **Type safety** - proper UUID handling throughout - -### Implementation Strategy - -#### Phase 1: Workflow Consolidation ✅ COMPLETED -- [x] **Remove DBOS dependency** - eliminated dbos_config.py and all DBOS imports -- [x] **Create unified TenantDeploymentWorkflow** - handles both provisioning and updates -- [x] **Remove legacy workflows** - deleted tenant_provisioning.py, tenant_update.py -- [x] **Simplify API endpoints** - consolidated to single `/deploy` endpoint -- [x] **Update integration tests** - comprehensive edge case testing - -#### Phase 2: Workflow Tracking System ✅ COMPLETED -- [x] **Database migration** - added workflow table and event.workflow_id foreign key -- [x] **Workflow repository** - CRUD operations for workflow records -- [x] **Event correlation** - all workflow events tagged with workflow_id -- [x] **Comprehensive testing** - workflow lifecycle and event grouping tests - -#### Phase 3: Parameter Standardization ✅ COMPLETED -- [x] **Standardize method signatures** - workflow_id as first parameter pattern -- [x] **Fix event tagging** - ensure all workflow events properly correlated -- [x] **Update service methods** - consistent parameter order across tenant_service -- [x] **Integration test validation** - verify complete event sequences - -### Architectural Benefits - -#### Code Simplification -- **39 files changed**: 2,247 additions, 3,256 deletions (net -1,009 lines) -- **Eliminated framework complexity** - no more DBOS configuration or abstractions -- **Consolidated logic** - single deployment workflow vs 4 separate workflows -- **Cleaner API surface** - unified endpoint vs multiple workflow-specific endpoints - -#### Enhanced Observability -- **Complete event correlation** - every workflow event tagged with workflow_id -- **Audit trail reconstruction** - can trace entire tenant lifecycle through events -- **Workflow status tracking** - running/completed/failed states in database -- **Comprehensive testing** - edge cases covered with real infrastructure - -#### Operational Benefits -- **Simpler debugging** - plain Python stack traces vs framework abstractions -- **Reduced dependencies** - one less complex framework to maintain -- **Better error handling** - explicit exception handling vs framework magic -- **Easier maintenance** - straightforward Python code vs orchestration complexity - -## How to Evaluate - -### Success Criteria - -#### Functional Completeness ✅ VERIFIED -- [x] **Unified deployment workflow** handles both initial provisioning and updates -- [x] **Undeploy workflow** properly integrated with event tracking -- [x] **All operations idempotent** - safe to retry any step without duplication -- [x] **Complete tenant lifecycle** - provision → active → update → undeploy - -#### Event Tracking and Correlation ✅ VERIFIED -- [x] **All workflow events tagged** with proper workflow_id -- [x] **Event sequence verification** - tests assert exact event order and content -- [x] **Workflow grouping** - events can be queried by workflow_id for complete audit trail -- [x] **Cross-workflow isolation** - deployment vs undeploy events properly separated - -#### Database Schema and Performance ✅ VERIFIED -- [x] **Migration applied** - workflow table and event.workflow_id column created -- [x] **Proper indexing** - performance optimized queries on workflow_type, tenant_id, status -- [x] **Foreign key constraints** - referential integrity between workflows and events -- [x] **Database triggers** - updated_at timestamp automation - -#### Test Coverage ✅ COMPREHENSIVE -- [x] **Unit tests**: 4 workflow tracking tests covering lifecycle and event grouping -- [x] **Integration tests**: Real infrastructure testing with Fly.io resources -- [x] **Edge case coverage**: Failed deployments, partial state recovery, resource conflicts -- [x] **Event sequence verification**: Exact event order and content validation - -### Testing Procedure - -#### Unit Test Validation ✅ PASSING -```bash -cd apps/cloud && pytest tests/test_workflow_tracking.py -v -# 4/4 tests passing - workflow lifecycle and event grouping -``` - -#### Integration Test Validation ✅ PASSING -```bash -cd apps/cloud && pytest tests/integration/test_tenant_workflow_deployment_integration.py -v -cd apps/cloud && pytest tests/integration/test_tenant_workflow_undeploy_integration.py -v -# Comprehensive real infrastructure testing with actual Fly.io resources -# Tests provision → deploy → update → undeploy → cleanup cycles -``` - -### Performance Metrics - -#### Code Metrics ✅ ACHIEVED -- **Net code reduction**: -1,009 lines (3,256 deletions, 2,247 additions) -- **Workflow consolidation**: 4 workflows → 1 unified deployment workflow -- **Dependency reduction**: Removed DBOS framework dependency entirely -- **API simplification**: Multiple endpoints → single `/deploy` endpoint - -#### Operational Metrics ✅ VERIFIED -- **Event correlation**: 100% of workflow events properly tagged with workflow_id -- **Audit trail completeness**: Full tenant lifecycle traceable through event sequences -- **Error handling**: Clean Python exceptions vs framework abstractions -- **Debugging simplicity**: Direct stack traces vs orchestration complexity - -### Implementation Status: ✅ COMPLETE - -All phases completed successfully with comprehensive testing and verification: - -**Phase 1 - Workflow Consolidation**: ✅ COMPLETE -- Removed DBOS dependency and consolidated workflows -- Unified deployment workflow handles all scenarios -- Comprehensive integration testing with real infrastructure - -**Phase 2 - Workflow Tracking**: ✅ COMPLETE -- Database schema implemented with proper indexing -- Event correlation system fully functional -- Complete audit trail capability verified - -**Phase 3 - Parameter Standardization**: ✅ COMPLETE -- Consistent method signatures across all workflow methods -- All events properly tagged with workflow_id -- Type safety verified across entire codebase - -**Phase 4 - Asynchronous Job Queuing**: -**Goal**: Transform synchronous deployment workflows into background jobs for better user experience and system reliability. - -**Current Problem**: -- Deployment API calls are synchronous - users wait for entire tenant provisioning (30-60 seconds) -- No retry mechanism for failed operations -- HTTP timeouts on long-running deployments -- Poor user experience during infrastructure provisioning - -**Solution**: Redis-backed job queue with arq for reliable background processing - -#### Architecture Overview -```python -# API Layer: Return immediately with job tracking -@router.post("/{tenant_id}/deploy") -async def deploy_tenant(tenant_id: UUID): - # Create workflow record in Postgres - workflow = await workflow_repo.create_workflow("tenant_deployment", tenant_id) - - # Enqueue job in Redis - job = await arq_pool.enqueue_job('deploy_tenant_task', tenant_id, workflow.id) - - # Return job ID immediately - return {"job_id": job.job_id, "workflow_id": workflow.id, "status": "queued"} - -# Background Worker: Process via existing unified workflow -async def deploy_tenant_task(ctx, tenant_id: str, workflow_id: str): - # Existing workflow logic - zero changes needed! - await workflow_manager.deploy_tenant(UUID(tenant_id), workflow_id=UUID(workflow_id)) -``` - -#### Implementation Tasks - -**Phase 4.1: Core Job Queue Setup** ✅ COMPLETED -- [x] **Add arq dependency** - integrated Redis job queue with existing infrastructure -- [x] **Create job definitions** - wrapped existing deployment/undeploy workflows as arq tasks -- [x] **Update API endpoints** - updated provisioning endpoints to return job IDs instead of waiting for completion -- [x] **JobQueueService implementation** - service layer for job enqueueing and status tracking -- [x] **Job status tracking** - integrated with existing workflow table for status updates -- [x] **Comprehensive testing** - 18 tests covering positive, negative, and edge cases - -**Phase 4.2: Background Worker Implementation** ✅ COMPLETED -- [x] **Job status API** - GET /jobs/{job_id}/status endpoint integrated with JobQueueService -- [x] **Background worker process** - arq worker to process queued jobs with proper settings and Redis configuration -- [x] **Worker settings and configuration** - WorkerSettings class with proper timeouts, max jobs, and error handling -- [x] **Fix API endpoints** - updated job status API to use JobQueueService instead of direct Redis access -- [x] **Integration testing** - comprehensive end-to-end testing with real ARQ workers and Fly.io infrastructure -- [x] **Worker entry points** - dual-purpose entrypoint.sh script and __main__.py module support for both API and worker processes -- [x] **Test fixture updates** - fixed all API and service test fixtures to work with job queue dependencies -- [x] **AsyncIO event loop fixes** - resolved event loop issues in integration tests for subprocess worker compatibility -- [x] **Complete test coverage** - all 46 tests passing across unit, integration, and API test suites -- [x] **Type safety verification** - 0 type checking errors across entire ARQ job queue implementation - -#### Phase 4.2 Implementation Summary ✅ COMPLETE - -**Core ARQ Job Queue System:** -- **JobQueueService** - Centralized service for job enqueueing, status tracking, and Redis pool management -- **deployment_jobs.py** - ARQ job functions that wrap existing deployment/undeploy workflows -- **Worker Settings** - Production-ready ARQ configuration with proper timeouts and error handling -- **Dual-Process Architecture** - Single Docker image with entrypoint.sh supporting both API and worker modes - -**Key Files Added:** -- `apps/cloud/src/basic_memory_cloud/jobs/` - Complete job queue implementation (7 files) -- `apps/cloud/entrypoint.sh` - Dual-purpose Docker container entry point -- `apps/cloud/tests/integration/test_worker_integration.py` - Real infrastructure integration tests -- `apps/cloud/src/basic_memory_cloud/schemas/job_responses.py` - API response schemas - -**API Integration:** -- Provisioning endpoints return job IDs immediately instead of blocking for 60+ seconds -- Job status API endpoints for real-time monitoring of deployment progress -- Proper error handling and job failure scenarios with detailed error messages - -**Testing Achievement:** -- **46 total tests passing** across all test suites (unit, integration, API, services) -- **Real infrastructure testing** - ARQ workers process actual Fly.io deployments -- **Event loop safety** - Fixed asyncio issues for subprocess worker compatibility -- **Test fixture updates** - All fixtures properly support job queue dependencies -- **Type checking** - 0 errors across entire codebase - -**Technical Metrics:** -- **38 files changed** - +1,736 insertions, -334 deletions -- **Integration test runtime** - ~18 seconds with real ARQ workers and Fly.io verification -- **Event loop isolation** - Proper async session management for subprocess compatibility -- **Redis integration** - Production-ready Redis configuration with connection pooling - -**Phase 4.3: Production Hardening** ✅ COMPLETED -- [x] **Configure Upstash Redis** - production Redis setup on Fly.io -- [x] **Retry logic for external APIs** - exponential backoff for flaky Tigris IAM operations -- [x] **Monitoring and observability** - comprehensive Redis queue monitoring with CLI tools -- [x] **Error handling improvements** - graceful handling of expected API errors with appropriate log levels -- [x] **CLI tooling enhancements** - bulk update commands for CI/CD automation -- [x] **Documentation improvements** - comprehensive monitoring guide with Redis patterns -- [x] **Job uniqueness** - ARQ-based duplicate prevention for tenant operations -- [ ] **Worker scaling** - multiple arq workers for parallel job processing -- [ ] **Job persistence** - ensure jobs survive Redis/worker restarts -- [ ] **Error alerting** - notifications for failed deployment jobs - -**Phase 4.4: Advanced Features** (Future) -- [ ] **Job scheduling** - deploy tenants at specific times -- [ ] **Priority queues** - urgent deployments processed first -- [ ] **Batch operations** - bulk tenant deployments -- [ ] **Job dependencies** - deployment → configuration → activation chains - -#### Benefits Achieved ✅ REALIZED - -**User Experience Improvements:** -- **Immediate API responses** - users get job ID instantly vs waiting 60+ seconds for deployment completion -- **Real-time job tracking** - status API provides live updates on deployment progress -- **Better error visibility** - detailed error messages and job failure tracking -- **CI/CD automation ready** - bulk update commands for automated tenant deployments - -**System Reliability:** -- **Redis persistence** - jobs survive Redis/worker restarts with proper queue durability -- **Idempotent job processing** - jobs can be safely retried without side effects -- **Event loop isolation** - worker processes operate independently from API server -- **Retry resilience** - exponential backoff for flaky external API calls (3 attempts, 1s/2s delays) -- **Graceful error handling** - expected API errors logged at INFO level, unexpected at ERROR level -- **Job uniqueness** - prevent duplicate tenant operations with ARQ's built-in uniqueness feature - -**Operational Benefits:** -- **Horizontal scaling ready** - architecture supports adding more workers for parallel processing -- **Comprehensive testing** - real infrastructure integration tests ensure production reliability -- **Type safety** - full type checking prevents runtime errors in job processing -- **Clean separation** - API and worker processes use same codebase with different entry points -- **Queue monitoring** - Redis CLI integration for real-time queue activity monitoring -- **Comprehensive documentation** - detailed monitoring guide with Redis pattern explanations - -**Development Benefits:** -- **Zero workflow changes** - existing deployment/undeploy workflows work unchanged as background jobs -- **Async/await native** - modern Python asyncio patterns throughout the implementation -- **Event correlation preserved** - all existing workflow tracking and event sourcing continues to work -- **Enhanced CLI tooling** - unified tenant commands with proper endpoint routing -- **Database integrity** - proper foreign key constraint handling in tenant deletion - -#### Infrastructure Requirements -- **Local**: Redis via docker-compose (already exists) ✅ -- **Production**: Upstash Redis on Fly.io (already configured) ✅ -- **Workers**: arq worker processes (new deployment target) -- **Monitoring**: Job status dashboard (simple web interface) - -#### API Evolution -```python -# Before: Synchronous (blocks for 60+ seconds) -POST /tenant/{id}/deploy → {status: "active", machine_id: "..."} - -# After: Asynchronous (returns immediately) -POST /tenant/{id}/deploy → {job_id: "uuid", workflow_id: "uuid", status: "queued"} -GET /jobs/{job_id}/status → {status: "running", progress: "deploying_machine", workflow_id: "uuid"} -GET /workflows/{workflow_id}/events → [...] # Existing event tracking works unchanged -``` - -**Technology Choice**: **arq (Redis)** over pgqueuer -- **Existing Redis infrastructure** - Upstash + docker-compose already configured -- **Better ecosystem** - monitoring tools, documentation, community -- **Made by pydantic team** - aligns with existing Python stack -- **Hybrid approach** - Redis for queue operations + Postgres for workflow state - -#### Job Uniqueness Implementation - -**Problem**: Multiple concurrent deployment requests for the same tenant could create duplicate jobs, wasting resources and potentially causing conflicts. - -**Solution**: Leverage ARQ's built-in job uniqueness feature using predictable job IDs: - -```python -# JobQueueService implementation -async def enqueue_deploy_job(self, tenant_id: UUID, image_tag: str | None = None) -> str: - unique_job_id = f"deploy-{tenant_id}" - - job = await self.redis_pool.enqueue_job( - "deploy_tenant_job", - str(tenant_id), - image_tag, - _job_id=unique_job_id, # ARQ prevents duplicates - ) - - if job is None: - # Job already exists - return existing job ID - return unique_job_id - else: - # New job created - return ARQ job ID - return job.job_id -``` - -**Key Features:** -- **Predictable Job IDs**: `deploy-{tenant_id}`, `undeploy-{tenant_id}` -- **Duplicate Prevention**: ARQ returns `None` for duplicate job IDs -- **Graceful Handling**: Return existing job ID instead of raising errors -- **Idempotent Operations**: Safe to retry deployment requests -- **Clear Logging**: Distinguish "Enqueued new" vs "Found existing" jobs - -**Benefits:** -- Prevents resource waste from duplicate deployments -- Eliminates race conditions from concurrent requests -- Makes job monitoring more predictable with consistent IDs -- Provides natural deduplication without complex locking mechanisms - - -## Notes - -### Design Philosophy Lessons -- **Simplicity beats framework magic** - removing DBOS made the system more reliable and debuggable -- **Event sourcing > complex orchestration** - database-backed event tracking provides better observability than framework abstractions -- **Idempotent operations > resumable workflows** - each step handling its own retry logic is simpler than framework-managed resumability -- **Explicit error handling > framework exception handling** - Python exceptions are clearer than orchestration framework error states - -### Future Considerations -- **Monitoring integration** - workflow tracking events could feed into observability systems -- **Performance optimization** - event querying patterns may benefit from additional indexing -- **Audit compliance** - complete event trail supports regulatory requirements -- **Operational dashboards** - workflow status could drive tenant health monitoring - -### Related Specifications -- **SPEC-8**: TigrisFS Integration - bucket provisioning integrated with deployment workflow -- **SPEC-1**: Specification-Driven Development Process - this spec follows the established format - -## Observations - -- [architecture] Removing framework complexity led to more maintainable system #simplification -- [workflow] Single unified deployment workflow handles both provisioning and updates #consolidation -- [observability] Event sourcing with workflow correlation provides complete audit trail #event-tracking -- [database] Foreign key relationships between workflows and events enable powerful queries #schema-design -- [testing] Integration tests with real infrastructure catch edge cases that unit tests miss #testing-strategy -- [parameters] Consistent method signatures (workflow_id first) reduce cognitive overhead #api-design -- [maintenance] Fewer workflows and dependencies reduce long-term maintenance burden #operational-excellence -- [debugging] Plain Python exceptions are clearer than framework abstraction layers #developer-experience -- [resilience] Exponential backoff retry patterns handle flaky external API calls gracefully #error-handling -- [monitoring] Redis queue monitoring provides real-time operational visibility #observability -- [ci-cd] Bulk update commands enable automated tenant deployments in continuous delivery pipelines #automation -- [documentation] Comprehensive monitoring guides reduce operational learning curve #knowledge-management -- [error-logging] Context-aware log levels (INFO for expected errors, ERROR for unexpected) improve signal-to-noise ratio #logging-strategy -- [job-uniqueness] ARQ job uniqueness with predictable tenant-based IDs prevents duplicate operations and resource waste #deduplication - -## Implementation Notes - -### Configuration Integration -- **Redis Configuration**: Add Redis settings to existing `apps/cloud/src/basic_memory_cloud/config.py` -- **Local Development**: Leverage existing Redis setup from `docker-compose.yml` -- **Production**: Use Upstash Redis configuration for production environments - -### Docker Entrypoint Strategy -Create `entrypoint.sh` script to toggle between API server and worker processes using single Docker image: - -```bash -#!/bin/bash - -# Entrypoint script for Basic Memory Cloud service -# Supports multiple process types: api, worker - -set -e - -case "$1" in - "api") - echo "Starting Basic Memory Cloud API server..." - exec uvicorn basic_memory_cloud.main:app \ - --host 0.0.0.0 \ - --port 8000 \ - --log-level info - ;; - "worker") - echo "Starting Basic Memory Cloud ARQ worker..." - # For ARQ worker implementation - exec python -m arq basic_memory_cloud.jobs.settings.WorkerSettings - ;; - *) - echo "Usage: $0 {api|worker}" - echo " api - Start the FastAPI server" - echo " worker - Start the ARQ worker" - exit 1 - ;; -esac -``` - -### Fly.io Process Groups Configuration -Use separate machine groups for API and worker processes with independent scaling: - -```toml -# fly.toml app configuration for basic-memory-cloud -app = 'basic-memory-cloud-dev-basic-machines' -primary_region = 'dfw' -org = 'basic-machines' -kill_signal = 'SIGINT' -kill_timeout = '5s' - -[build] - -# Process groups for API server and worker -[processes] - api = "api" - worker = "worker" - -# Machine scaling configuration -[[machine]] - size = 'shared-cpu-1x' - processes = ['api'] - min_machines_running = 1 - auto_stop_machines = false - auto_start_machines = true - -[[machine]] - size = 'shared-cpu-1x' - processes = ['worker'] - min_machines_running = 1 - auto_stop_machines = false - auto_start_machines = true - -[env] - # Python configuration - PYTHONUNBUFFERED = '1' - PYTHONPATH = '/app' - - # Logging configuration - LOG_LEVEL = 'DEBUG' - - # Redis configuration for ARQ - REDIS_URL = 'redis://basic-memory-cloud-redis.upstash.io' - - # Database configuration - DATABASE_HOST = 'basic-memory-cloud-db-dev-basic-machines.internal' - DATABASE_PORT = '5432' - DATABASE_NAME = 'basic_memory_cloud' - DATABASE_USER = 'postgres' - DATABASE_SSL = 'true' - - # Worker configuration - ARQ_MAX_JOBS = '10' - ARQ_KEEP_RESULT = '3600' - - # Fly.io configuration - FLY_ORG = 'basic-machines' - FLY_REGION = 'dfw' - -# Internal service - no external HTTP exposure for worker -# API accessible via basic-memory-cloud-dev-basic-machines.flycast:8000 - -[[vm]] - size = 'shared-cpu-1x' -``` - -### Benefits of This Architecture -- **Single Docker Image**: Both API and worker use same container with different entrypoints -- **Independent Scaling**: Scale API and worker processes separately based on demand -- **Clean Separation**: Web traffic handling separate from background job processing -- **Existing Infrastructure**: Leverages current PostgreSQL + Redis setup without complexity -- **Hybrid State Management**: Redis for queue operations, PostgreSQL for persistent workflow tracking - -## Relations - -- implements [[SPEC-8 TigrisFS Integration]] -- follows [[SPEC-1 Specification-Driven Development Process]] -- supersedes previous multi-workflow architecture diff --git a/specs/SPEC-11 Basic Memory API Performance Optimization.md b/specs/SPEC-11 Basic Memory API Performance Optimization.md deleted file mode 100644 index 79779533..00000000 --- a/specs/SPEC-11 Basic Memory API Performance Optimization.md +++ /dev/null @@ -1,186 +0,0 @@ ---- -title: 'SPEC-11: Basic Memory API Performance Optimization' -type: spec -permalink: specs/spec-11-basic-memory-api-performance-optimization -tags: -- performance -- api -- mcp -- database -- cloud ---- - -# SPEC-11: Basic Memory API Performance Optimization - -## Why - -The Basic Memory API experiences significant performance issues in cloud environments due to expensive per-request initialization. MCP tools making -HTTP requests to the API suffer from 350ms-2.6s latency overhead **before** any actual operation occurs. - -**Root Cause Analysis:** -- GitHub Issue #82 shows repeated initialization sequences in logs (16:29:35 and 16:49:58) -- Each MCP tool call triggers full database initialization + project reconciliation -- `get_engine_factory()` dependency calls `db.get_or_create_db()` on every request -- `reconcile_projects_with_config()` runs expensive sync operations repeatedly - -**Performance Impact:** -- Database connection setup: ~50-100ms per request -- Migration checks: ~100-500ms per request -- Project reconciliation: ~200ms-2s per request -- **Total overhead**: ~350ms-2.6s per MCP tool call - -This creates compounding effects with tenant auto-start delays and increases timeout risk in cloud deployments. - -## What - -This optimization affects the **core basic-memory repository** components: - -1. **API Lifespan Management** (`src/basic_memory/api/app.py`) - - Cache database connections in app state during startup - - Avoid repeated expensive initialization - -2. **Dependency Injection** (`src/basic_memory/deps.py`) - - Modify `get_engine_factory()` to use cached connections - - Eliminate per-request database setup - -3. **Initialization Service** (`src/basic_memory/services/initialization.py`) - - Add caching/throttling to project reconciliation - - Skip expensive operations when appropriate - -4. **Configuration** (`src/basic_memory/config.py`) - - Add optional performance flags for cloud environments - -**Backwards Compatibility**: All changes must be backwards compatible with existing CLI and non-cloud usage. - -## How (High Level) - -### Phase 1: Cache Database Connections (Critical - 80% of gains) - -**Problem**: `get_engine_factory()` calls `db.get_or_create_db()` per request -**Solution**: Cache database engine/session in app state during lifespan - -1. **Modify API Lifespan** (`api/app.py`): - ```python - @asynccontextmanager - async def lifespan(app: FastAPI): - app_config = ConfigManager().config - await initialize_app(app_config) - - # Cache database connection in app state - engine, session_maker = await db.get_or_create_db(app_config.database_path) - app.state.engine = engine - app.state.session_maker = session_maker - - # ... rest of startup logic -``` - -2. Modify Dependency Injection (deps.py): -```python -async def get_engine_factory( - request: Request -) -> tuple[AsyncEngine, async_sessionmaker[AsyncSession]]: - """Get cached engine and session maker from app state.""" - return request.app.state.engine, request.app.state.session_maker -``` -Phase 2: Optimize Project Reconciliation (Secondary - 20% of gains) - -Problem: reconcile_projects_with_config() runs expensive sync repeatedly -Solution: Add module-level caching with time-based throttling - -1. Add Reconciliation Cache (services/initialization.py): -```ptyhon -_project_reconciliation_completed = False -_last_reconciliation_time = 0 - -async def reconcile_projects_with_config(app_config, force=False): - # Skip if recently completed (within 60 seconds) unless forced - if recently_completed and not force: - return - # ... existing logic -``` -Phase 3: Cloud Environment Flags (Optional) - -Problem: Force expensive initialization in production environments -Solution: Add skip flags for cloud/stateless deployments - -1. Add Config Flag (config.py): -skip_initialization_sync: bool = Field(default=False) -2. Configure in Cloud (basic-memory-cloud integration): -BASIC_MEMORY_SKIP_INITIALIZATION_SYNC=true - -How to Evaluate - -Success Criteria - -1. Performance Metrics (Primary): -- MCP tool response time reduced by 50%+ (measure before/after) -- Database connection overhead eliminated (0ms vs 50-100ms) -- Migration check overhead eliminated (0ms vs 100-500ms) -- Project reconciliation overhead reduced by 90%+ -2. Load Testing: -- Concurrent MCP tool calls maintain performance -- No memory leaks in cached connections -- Database connection pool behaves correctly -3. Functional Correctness: -- All existing API endpoints work identically -- MCP tools maintain full functionality -- CLI operations unaffected -- Database migrations still execute properly -4. Backwards Compatibility: -- No breaking changes to existing APIs -- Config changes are optional with safe defaults -- Non-cloud deployments work unchanged - -Testing Strategy - -Performance Testing: -# Before optimization -time basic-memory-mcp-tools write_note "test" "content" "folder" -# Measure: ~1-3 seconds - -# After optimization -time basic-memory-mcp-tools write_note "test" "content" "folder" -# Target: <500ms - -Load Testing: -# Multiple concurrent MCP tool calls -for i in {1..10}; do -basic-memory-mcp-tools search "test" & -done -wait -# Verify: No degradation, consistent response times - -Regression Testing: -# Full basic-memory test suite -just test -# All tests must pass - -# Integration tests with cloud deployment -# Verify MCP gateway → API → database flow works - -Validation Checklist - -- Phase 1 Complete: Database connections cached, dependency injection optimized -- Performance Benchmark: 50%+ improvement in MCP tool response times -- Memory Usage: No leaks in cached connections over 24h+ periods -- Stress Testing: 100+ concurrent requests maintain performance -- Backwards Compatibility: All existing functionality preserved -- Documentation: Performance optimization documented in README -- Cloud Integration: basic-memory-cloud sees performance benefits - -Notes - -Implementation Priority: -- Phase 1 provides 80% of performance gains and should be implemented first -- Phase 2 provides remaining 20% and addresses edge cases -- Phase 3 is optional for maximum cloud optimization - -Risk Mitigation: -- All changes backwards compatible -- Gradual rollout possible (Phase 1 → 2 → 3) -- Easy rollback via configuration flags - -Cloud Integration: -- This optimization directly addresses basic-memory-cloud issue #82 -- Changes in core basic-memory will benefit all cloud tenants -- No changes needed in basic-memory-cloud itself diff --git a/specs/SPEC-12 OpenTelemetry Observability.md b/specs/SPEC-12 OpenTelemetry Observability.md deleted file mode 100644 index e38d52fe..00000000 --- a/specs/SPEC-12 OpenTelemetry Observability.md +++ /dev/null @@ -1,182 +0,0 @@ -# SPEC-12: OpenTelemetry Observability - -## Why - -We need comprehensive observability for basic-memory-cloud to: -- Track request flows across our multi-tenant architecture (MCP → Cloud → API services) -- Debug performance issues and errors in production -- Understand user behavior and system usage patterns -- Correlate issues to specific tenants for targeted debugging -- Monitor service health and latency across the distributed system - -Currently, we only have basic logging without request correlation or distributed tracing capabilities. - -## What - -Implement OpenTelemetry instrumentation across all basic-memory-cloud services with: - -### Core Requirements -1. **Distributed Tracing**: End-to-end request tracing from MCP gateway through to tenant API instances -2. **Tenant Correlation**: All traces tagged with tenant_id, user_id, and workos_user_id -3. **Service Identification**: Clear service naming and namespace separation -4. **Auto-instrumentation**: Automatic tracing for FastAPI, SQLAlchemy, HTTP clients -5. **Grafana Cloud Integration**: Direct OTLP export to Grafana Cloud Tempo - -### Services to Instrument -- **MCP Gateway** (basic-memory-mcp): Entry point with JWT extraction -- **Cloud Service** (basic-memory-cloud): Provisioning and management operations -- **API Service** (basic-memory-api): Tenant-specific instances -- **Worker Processes** (ARQ workers): Background job processing - -### Key Trace Attributes -- `tenant.id`: UUID from UserProfile.tenant_id -- `user.id`: WorkOS user identifier -- `user.email`: User email for debugging -- `service.name`: Specific service identifier -- `service.namespace`: Environment (development/production) -- `operation.type`: Business operation (provision/update/delete) -- `tenant.app_name`: Fly.io app name for tenant instances - -## How - -### Phase 1: Setup OpenTelemetry SDK -1. Add OpenTelemetry dependencies to each service's pyproject.toml: - ```python - "opentelemetry-distro[otlp]>=1.29.0", - "opentelemetry-instrumentation-fastapi>=0.50b0", - "opentelemetry-instrumentation-httpx>=0.50b0", - "opentelemetry-instrumentation-sqlalchemy>=0.50b0", - "opentelemetry-instrumentation-logging>=0.50b0", - ``` - -2. Create shared telemetry initialization module (`apps/shared/telemetry.py`) - -3. Configure Grafana Cloud OTLP endpoint via environment variables: - ```bash - OTEL_EXPORTER_OTLP_ENDPOINT=https://otlp-gateway-prod-us-east-2.grafana.net/otlp - OTEL_EXPORTER_OTLP_HEADERS=Authorization=Basic[token] - OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf - ``` - -### Phase 2: Instrument MCP Gateway -1. Extract tenant context from AuthKit JWT in middleware -2. Create root span with tenant attributes -3. Propagate trace context to downstream services via headers - -### Phase 3: Instrument Cloud Service -1. Continue trace from MCP gateway -2. Add operation-specific attributes (provisioning events) -3. Instrument ARQ worker jobs for async operations -4. Track Fly.io API calls and latency - -### Phase 4: Instrument API Service -1. Extract tenant context from JWT -2. Add machine-specific metadata (instance ID, region) -3. Instrument database operations with SQLAlchemy -4. Track MCP protocol operations - -### Phase 5: Configure and Deploy -1. Add OTLP configuration to `.env.example` and `.env.example.secrets` -2. Set Fly.io secrets for production deployment -3. Update Dockerfiles to use `opentelemetry-instrument` wrapper -4. Deploy to development environment first for testing - -## How to Evaluate - -### Success Criteria -1. **End-to-end traces visible in Grafana Cloud** showing complete request flow -2. **Tenant filtering works** - Can filter traces by tenant_id to see all requests for a user -3. **Service maps accurate** - Grafana shows correct service dependencies -4. **Performance overhead < 5%** - Minimal latency impact from instrumentation -5. **Error correlation** - Can trace errors back to specific tenant and operation - -### Testing Checklist -- [x] Single request creates connected trace across all services -- [x] Tenant attributes present on all spans -- [x] Background jobs (ARQ) appear in traces -- [x] Database queries show in trace timeline -- [x] HTTP calls to Fly.io API tracked -- [x] Traces exported successfully to Grafana Cloud -- [x] Can search traces by tenant_id in Grafana -- [x] Service dependency graph shows correct flow - -### Monitoring Success -- All services reporting traces to Grafana Cloud -- No OTLP export errors in logs -- Trace sampling working correctly (if implemented) -- Resource usage acceptable (CPU/memory) - -## Dependencies -- Grafana Cloud account with OTLP endpoint configured -- OpenTelemetry Python SDK v1.29.0+ -- FastAPI instrumentation compatibility -- Network access from Fly.io to Grafana Cloud - -## Implementation Assignment -**Recommended Agent**: python-developer -- Requires Python/FastAPI expertise -- Needs understanding of distributed systems -- Must implement middleware and context propagation -- Should understand OpenTelemetry SDK and instrumentation - -## Follow-up Tasks - -### Enhanced Log Correlation -While basic trace-to-log correlation works automatically via OpenTelemetry logging instrumentation, consider adding structured logging for improved log filtering: - -1. **Structured Logging Context**: Add `logger.bind()` calls to inject tenant/user context directly into log records -2. **Custom Loguru Formatter**: Extract OpenTelemetry span attributes for better log readability -3. **Direct Log Filtering**: Enable searching logs directly by tenant_id, workflow_id without going through traces - -This would complement the existing automatic trace correlation and provide better log search capabilities. - -## Alternative Solution: Logfire - -After implementing OpenTelemetry with Grafana Cloud, we discovered limitations in the observability experience: -- Traces work but lack useful context without correlated logs -- Setting up log correlation with Grafana is complex and requires additional infrastructure -- The developer experience for Python observability is suboptimal - -### Logfire Evaluation - -**Pydantic Logfire** offers a compelling alternative that addresses your specific requirements: - -#### Core Requirements Match -- ✅ **User Activity Tracking**: Automatic request tracing with business context -- ✅ **Error Monitoring**: Built-in exception tracking with full context -- ✅ **Performance Metrics**: Automatic latency and performance monitoring -- ✅ **Request Tracing**: Native distributed tracing across services -- ✅ **Log Correlation**: Seamless trace-to-log correlation without setup - -#### Key Advantages -1. **Python-First Design**: Built specifically for Python/FastAPI applications by the Pydantic team -2. **Simple Integration**: `pip install logfire` + `logfire.configure()` vs complex OTLP setup -3. **Automatic Correlation**: Logs automatically include trace context without manual configuration -4. **Real-time SQL Interface**: Query spans and logs using SQL with auto-completion -5. **Better Developer UX**: Purpose-built observability UI vs generic Grafana dashboards -6. **Loguru Integration**: `logger.configure(handlers=[logfire.loguru_handler()])` maintains existing logging - -#### Pricing Assessment -- **Free Tier**: 10M spans/month (suitable for development and small production workloads) -- **Transparent Pricing**: $1 per million spans/metrics after free tier -- **No Hidden Costs**: No per-host fees, only usage-based metering -- **Production Ready**: Recently exited beta, enterprise features available - -#### Migration Path -The existing OpenTelemetry instrumentation is compatible - Logfire uses OpenTelemetry under the hood, so the current spans and attributes would work unchanged. - -### Recommendation - -**Consider migrating to Logfire** for the following reasons: -1. It directly addresses the "next to useless" traces problem by providing integrated logs -2. Dramatically simpler setup and maintenance compared to Grafana Cloud + custom log correlation -3. Better ROI on observability investment with purpose-built Python tooling -4. Free tier sufficient for current development needs with clear scaling path - -The current Grafana Cloud implementation provides a solid foundation and could remain as a backup/export target, while Logfire becomes the primary observability platform. - -## Status -**Created**: 2024-01-28 -**Status**: Completed (OpenTelemetry + Grafana Cloud) -**Next Phase**: Evaluate Logfire migration -**Priority**: High - Critical for production observability diff --git a/specs/SPEC-13 CLI Authentication with Subscription Validation.md b/specs/SPEC-13 CLI Authentication with Subscription Validation.md deleted file mode 100644 index 0f6ca8c5..00000000 --- a/specs/SPEC-13 CLI Authentication with Subscription Validation.md +++ /dev/null @@ -1,917 +0,0 @@ ---- -title: 'SPEC-13: CLI Authentication with Subscription Validation' -type: spec -permalink: specs/spec-12-cli-auth-subscription-validation -tags: -- authentication -- security -- cli -- subscription -status: draft -created: 2025-10-02 ---- - -# SPEC-13: CLI Authentication with Subscription Validation - -## Why - -The Basic Memory Cloud CLI currently has a security gap in authentication that allows unauthorized access: - -**Current Web Flow (Secure)**: -1. User signs up via WorkOS AuthKit -2. User creates Polar subscription -3. Web app validates subscription before calling `POST /tenants/setup` -4. Tenant provisioned only after subscription validation ✅ - -**Current CLI Flow (Insecure)**: -1. User signs up via WorkOS AuthKit (OAuth device flow) -2. User runs `bm cloud login` -3. CLI receives JWT token from WorkOS -4. CLI can access all cloud endpoints without subscription check ❌ - -**Problem**: Anyone can sign up with WorkOS and immediately access cloud infrastructure via CLI without having an active Polar subscription. This creates: -- Revenue loss (free resource consumption) -- Security risk (unauthorized data access) -- Support burden (users accessing features they haven't paid for) - -**Root Cause**: The CLI authentication flow validates JWT tokens but doesn't verify subscription status before granting access to cloud resources. - -## What - -Add subscription validation to authentication flow to ensure only users with active Polar subscriptions can access cloud resources across all access methods (CLI, MCP, Web App, Direct API). - -**Affected Components**: - -### basic-memory-cloud (Cloud Service) -- `apps/cloud/src/basic_memory_cloud/deps.py` - Add subscription validation dependency -- `apps/cloud/src/basic_memory_cloud/services/subscription_service.py` - Add subscription check method -- `apps/cloud/src/basic_memory_cloud/api/tenant_mount.py` - Protect mount endpoints -- `apps/cloud/src/basic_memory_cloud/api/proxy.py` - Protect proxy endpoints - -### basic-memory (CLI) -- `src/basic_memory/cli/commands/cloud/core_commands.py` - Handle 403 errors -- `src/basic_memory/cli/commands/cloud/api_client.py` - Parse subscription errors -- `docs/cloud-cli.md` - Document subscription requirement - -**Endpoints to Protect**: -- `GET /tenant/mount/info` - Used by CLI bisync setup -- `POST /tenant/mount/credentials` - Used by CLI bisync credentials -- `GET /proxy/{path:path}` - Used by Web App, MCP tools, CLI tools, Direct API -- All other `/proxy/*` endpoints - Centralized access point for all user operations - -## Complete Authentication Flow Analysis - -### Overview of All Access Flows - -Basic Memory Cloud has **7 distinct authentication flows**. This spec closes subscription validation gaps in flows 2-4 and 6, which all converge on the `/proxy/*` endpoints. - -### Flow 1: Polar Webhook → Registration ✅ SECURE -``` -Polar webhook → POST /api/webhooks/polar -→ Validates Polar webhook signature -→ Creates/updates subscription in database -→ No direct user access - webhook only -``` -**Auth**: Polar webhook signature validation -**Subscription Check**: N/A (webhook creates subscriptions) -**Status**: ✅ Secure - webhook validated, no user JWT involved - -### Flow 2: Web App Login ❌ NEEDS FIX -``` -User → apps/web (Vue.js/Nuxt) -→ WorkOS AuthKit magic link authentication -→ JWT stored in browser session -→ Web app calls /proxy/{project}/... endpoints (memory, directory, projects) -→ proxy.py validates JWT but does NOT check subscription -→ Access granted without subscription ❌ -``` -**Auth**: WorkOS JWT via `CurrentUserProfileHybridJwtDep` -**Subscription Check**: ❌ Missing -**Fixed By**: Task 1.4 (protect `/proxy/*` endpoints) - -### Flow 3: MCP (Model Context Protocol) ❌ NEEDS FIX -``` -AI Agent (Claude, Cursor, etc.) → https://mcp.basicmemory.com -→ AuthKit OAuth device flow -→ JWT stored in AI agent -→ MCP tools call {cloud_host}/proxy/{endpoint} with Authorization header -→ proxy.py validates JWT but does NOT check subscription -→ MCP tools can access all cloud resources without subscription ❌ -``` -**Auth**: AuthKit JWT via `CurrentUserProfileHybridJwtDep` -**Subscription Check**: ❌ Missing -**Fixed By**: Task 1.4 (protect `/proxy/*` endpoints) - -### Flow 4: CLI Auth (basic-memory) ❌ NEEDS FIX -``` -User → bm cloud login -→ AuthKit OAuth device flow -→ JWT stored in ~/.basic-memory/tokens.json -→ CLI calls: - - {cloud_host}/tenant/mount/info (for bisync setup) - - {cloud_host}/tenant/mount/credentials (for bisync credentials) - - {cloud_host}/proxy/{endpoint} (for all MCP tools) -→ tenant_mount.py and proxy.py validate JWT but do NOT check subscription -→ Access granted without subscription ❌ -``` -**Auth**: AuthKit JWT via `CurrentUserProfileHybridJwtDep` -**Subscription Check**: ❌ Missing -**Fixed By**: Task 1.3 (protect `/tenant/mount/*`) + Task 1.4 (protect `/proxy/*`) - -### Flow 5: Cloud CLI (Admin Tasks) ✅ SECURE -``` -Admin → python -m basic_memory_cloud.cli.tenant_cli -→ Uses CLIAuth with admin WorkOS OAuth client -→ Gets JWT token with admin org membership -→ Calls /tenants/* endpoints (create, list, delete tenants) -→ tenants.py validates JWT AND admin org membership via AdminUserHybridDep -→ Access granted only to admin organization members ✅ -``` -**Auth**: AuthKit JWT + Admin org validation via `AdminUserHybridDep` -**Subscription Check**: N/A (admins bypass subscription requirement) -**Status**: ✅ Secure - admin-only endpoints, separate from user flows - -### Flow 6: Direct API Calls ❌ NEEDS FIX -``` -Any HTTP client → {cloud_host}/proxy/{endpoint} -→ Sends Authorization: Bearer {jwt} header -→ proxy.py validates JWT but does NOT check subscription -→ Direct API access without subscription ❌ -``` -**Auth**: WorkOS or AuthKit JWT via `CurrentUserProfileHybridJwtDep` -**Subscription Check**: ❌ Missing -**Fixed By**: Task 1.4 (protect `/proxy/*` endpoints) - -### Flow 7: Tenant API Instance (Internal) ✅ SECURE -``` -/proxy/* → Tenant API (basic-memory-{tenant_id}.fly.dev) -→ Validates signed header from proxy (tenant_id + signature) -→ Direct external access will be disabled in production -→ Only accessible via /proxy endpoints -``` -**Auth**: Signed header validation from proxy -**Subscription Check**: N/A (internal only, validated at proxy layer) -**Status**: ✅ Secure - validates proxy signature, not directly accessible - -### Authentication Flow Summary Matrix - -| Flow | Access Method | Current Auth | Subscription Check | Fixed By SPEC-13 | -|------|---------------|--------------|-------------------|------------------| -| 1. Polar Webhook | Polar webhook → `/api/webhooks/polar` | Polar signature | N/A (webhook) | N/A | -| 2. Web App | Browser → `/proxy/*` | WorkOS JWT ✅ | ❌ Missing | ✅ Task 1.4 | -| 3. MCP | AI Agent → `/proxy/*` | AuthKit JWT ✅ | ❌ Missing | ✅ Task 1.4 | -| 4. CLI | `bm cloud` → `/tenant/mount/*` + `/proxy/*` | AuthKit JWT ✅ | ❌ Missing | ✅ Task 1.3 + 1.4 | -| 5. Cloud CLI (Admin) | `tenant_cli` → `/tenants/*` | AuthKit JWT ✅ + Admin org | N/A (admin) | N/A (admin bypass) | -| 6. Direct API | HTTP client → `/proxy/*` | WorkOS/AuthKit JWT ✅ | ❌ Missing | ✅ Task 1.4 | -| 7. Tenant API | Proxy → tenant instance | Proxy signature ✅ | N/A (internal) | N/A | - -### Key Insights - -1. **Single Point of Failure**: All user access (Web, MCP, CLI, Direct API) converges on `/proxy/*` endpoints -2. **Centralized Fix**: Protecting `/proxy/*` with subscription validation closes gaps in flows 2, 3, 4, and 6 simultaneously -3. **Admin Bypass**: Cloud CLI admin tasks use separate `/tenants/*` endpoints with admin-only access (no subscription needed) -4. **Defense in Depth**: `/tenant/mount/*` endpoints also protected for CLI bisync operations - -### Architecture Benefits - -The `/proxy` layer serves as the **single centralized authorization point** for all user access: -- ✅ One place to validate JWT tokens -- ✅ One place to check subscription status -- ✅ One place to handle tenant routing -- ✅ Protects Web App, MCP, CLI, and Direct API simultaneously - -This architecture makes the fix comprehensive and maintainable. - -## How (High Level) - -### Option A: Database Subscription Check (Recommended) - -**Approach**: Add FastAPI dependency that validates subscription status from database before allowing access. - -**Implementation**: - -1. **Create Subscription Validation Dependency** (`deps.py`) - ```python - async def get_authorized_cli_user_profile( - credentials: Annotated[HTTPAuthorizationCredentials, Depends(security)], - session: DatabaseSessionDep, - user_profile_repo: UserProfileRepositoryDep, - subscription_service: SubscriptionServiceDep, - ) -> UserProfile: - """ - Hybrid authentication with subscription validation for CLI access. - - Validates JWT (WorkOS or AuthKit) and checks for active subscription. - Returns UserProfile if both checks pass. - """ - # Try WorkOS JWT first (faster validation path) - try: - user_context = await validate_workos_jwt(credentials.credentials) - except HTTPException: - # Fall back to AuthKit JWT validation - try: - user_context = await validate_authkit_jwt(credentials.credentials) - except HTTPException as e: - raise HTTPException( - status_code=401, - detail="Invalid JWT token. Authentication required.", - ) from e - - # Check subscription status - has_subscription = await subscription_service.check_user_has_active_subscription( - session, user_context.workos_user_id - ) - - if not has_subscription: - raise HTTPException( - status_code=403, - detail={ - "error": "subscription_required", - "message": "Active subscription required for CLI access", - "subscribe_url": "https://basicmemory.com/subscribe" - } - ) - - # Look up and return user profile - user_profile = await user_profile_repo.get_user_profile_by_workos_user_id( - session, user_context.workos_user_id - ) - if not user_profile: - raise HTTPException(401, detail="User profile not found") - - return user_profile - ``` - - ```python - AuthorizedCLIUserProfileDep = Annotated[UserProfile, Depends(get_authorized_cli_user_profile)] - ``` - -2. **Add Subscription Check Method** (`subscription_service.py`) - ```python - async def check_user_has_active_subscription( - self, session: AsyncSession, workos_user_id: str - ) -> bool: - """Check if user has active subscription.""" - # Use existing repository method to get subscription by workos_user_id - # This joins UserProfile -> Subscription in a single query - subscription = await self.subscription_repository.get_subscription_by_workos_user_id( - session, workos_user_id - ) - - return subscription is not None and subscription.status == "active" - ``` - -3. **Protect Endpoints** (Replace `CurrentUserProfileHybridJwtDep` with `AuthorizedCLIUserProfileDep`) - ```python - # Before - @router.get("/mount/info") - async def get_mount_info( - user_profile: CurrentUserProfileHybridJwtDep, - session: DatabaseSessionDep, - ): - tenant_id = user_profile.tenant_id - ... - - # After - @router.get("/mount/info") - async def get_mount_info( - user_profile: AuthorizedCLIUserProfileDep, # Now includes subscription check - session: DatabaseSessionDep, - ): - tenant_id = user_profile.tenant_id # No changes needed to endpoint logic - ... - ``` - -4. **Update CLI Error Handling** - ```python - # In core_commands.py login() - try: - success = await auth.login() - if success: - # Test subscription by calling protected endpoint - await make_api_request("GET", f"{host_url}/tenant/mount/info") - except CloudAPIError as e: - if e.status_code == 403 and e.detail.get("error") == "subscription_required": - console.print("[red]Subscription required[/red]") - console.print(f"Subscribe at: {e.detail['subscribe_url']}") - raise typer.Exit(1) - ``` - -**Pros**: -- Simple to implement -- Fast (single database query) -- Clear error messages -- Works with existing subscription flow - -**Cons**: -- Database is source of truth (could get out of sync with Polar) -- Adds one extra subscription lookup query per request (lightweight JOIN query) - -### Option B: WorkOS Organizations - -**Approach**: Add users to "beta-users" organization in WorkOS after subscription creation, validate org membership via JWT claims. - -**Implementation**: -1. After Polar subscription webhook, add user to WorkOS org via API -2. Validate `org_id` claim in JWT matches authorized org -3. Use existing `get_admin_workos_jwt` pattern - -**Pros**: -- WorkOS as single source of truth -- No database queries needed -- More secure (harder to bypass) - -**Cons**: -- More complex (requires WorkOS API integration) -- Requires managing WorkOS org membership -- Less control over error messages -- Additional API calls during registration - -### Recommendation - -**Start with Option A (Database Check)** for: -- Faster implementation -- Clearer error messages -- Easier testing -- Existing subscription infrastructure - -**Consider Option B later** if: -- Need tighter security -- Want to reduce database dependency -- Scale requires fewer database queries - -## How to Evaluate - -### Success Criteria - -**1. Unauthorized Users Blocked** -- [ ] User without subscription cannot complete `bm cloud login` -- [ ] User without subscription receives clear error with subscribe link -- [ ] User without subscription cannot run `bm cloud setup` -- [ ] User without subscription cannot run `bm sync` in cloud mode - -**2. Authorized Users Work** -- [ ] User with active subscription can login successfully -- [ ] User with active subscription can setup bisync -- [ ] User with active subscription can sync files -- [ ] User with active subscription can use all MCP tools via proxy - -**3. Subscription State Changes** -- [ ] Expired subscription blocks access with clear error -- [ ] Renewed subscription immediately restores access -- [ ] Cancelled subscription blocks access after grace period - -**4. Error Messages** -- [ ] 403 errors include "subscription_required" error code -- [ ] Error messages include subscribe URL -- [ ] CLI displays user-friendly messages -- [ ] Errors logged appropriately for debugging - -**5. No Regressions** -- [ ] Web app login/subscription flow unaffected -- [ ] Admin endpoints still work (bypass check) -- [ ] Tenant provisioning workflow unchanged -- [ ] Performance not degraded - -### Test Cases - -**Manual Testing**: -```bash -# Test 1: Unauthorized user -1. Create new WorkOS account (no subscription) -2. Run `bm cloud login` -3. Verify: Login succeeds but shows subscription required error -4. Verify: Cannot run `bm cloud setup` -5. Verify: Clear error message with subscribe link - -# Test 2: Authorized user -1. Use account with active Polar subscription -2. Run `bm cloud login` -3. Verify: Login succeeds without errors -4. Run `bm cloud setup` -5. Verify: Setup completes successfully -6. Run `bm sync` -7. Verify: Sync works normally - -# Test 3: Subscription expiration -1. Use account with active subscription -2. Manually expire subscription in database -3. Run `bm cloud login` -4. Verify: Blocked with clear error -5. Renew subscription -6. Run `bm cloud login` again -7. Verify: Access restored -``` - -**Automated Tests**: -```python -# Test subscription validation dependency -async def test_authorized_user_allowed( - db_session, - user_profile_repo, - subscription_service, - mock_jwt_credentials -): - # Create user with active subscription - user_profile = await create_user_with_subscription(db_session, status="active") - - # Mock JWT credentials for the user - credentials = mock_jwt_credentials(user_profile.workos_user_id) - - # Should not raise exception - result = await get_authorized_cli_user_profile( - credentials, db_session, user_profile_repo, subscription_service - ) - assert result.id == user_profile.id - assert result.workos_user_id == user_profile.workos_user_id - -async def test_unauthorized_user_blocked( - db_session, - user_profile_repo, - subscription_service, - mock_jwt_credentials -): - # Create user without subscription - user_profile = await create_user_without_subscription(db_session) - credentials = mock_jwt_credentials(user_profile.workos_user_id) - - # Should raise 403 - with pytest.raises(HTTPException) as exc: - await get_authorized_cli_user_profile( - credentials, db_session, user_profile_repo, subscription_service - ) - - assert exc.value.status_code == 403 - assert exc.value.detail["error"] == "subscription_required" - -async def test_inactive_subscription_blocked( - db_session, - user_profile_repo, - subscription_service, - mock_jwt_credentials -): - # Create user with cancelled/inactive subscription - user_profile = await create_user_with_subscription(db_session, status="cancelled") - credentials = mock_jwt_credentials(user_profile.workos_user_id) - - # Should raise 403 - with pytest.raises(HTTPException) as exc: - await get_authorized_cli_user_profile( - credentials, db_session, user_profile_repo, subscription_service - ) - - assert exc.value.status_code == 403 - assert exc.value.detail["error"] == "subscription_required" -``` - -## Implementation Tasks - -### Phase 1: Cloud Service (basic-memory-cloud) - -#### Task 1.1: Add subscription check method to SubscriptionService ✅ -**File**: `apps/cloud/src/basic_memory_cloud/services/subscription_service.py` - -- [x] Add method `check_subscription(session: AsyncSession, workos_user_id: str) -> bool` -- [x] Use existing `self.subscription_repository.get_subscription_by_workos_user_id(session, workos_user_id)` -- [x] Check both `status == "active"` AND `current_period_end >= now()` -- [x] Log both values when check fails -- [x] Add docstring explaining the method -- [x] Run `just typecheck` to verify types - -**Actual implementation**: -```python -async def check_subscription( - self, session: AsyncSession, workos_user_id: str -) -> bool: - """Check if user has active subscription with valid period.""" - subscription = await self.subscription_repository.get_subscription_by_workos_user_id( - session, workos_user_id - ) - - if subscription is None: - return False - - if subscription.status != "active": - logger.warning("Subscription inactive", workos_user_id=workos_user_id, - status=subscription.status, current_period_end=subscription.current_period_end) - return False - - now = datetime.now(timezone.utc) - if subscription.current_period_end is None or subscription.current_period_end < now: - logger.warning("Subscription expired", workos_user_id=workos_user_id, - status=subscription.status, current_period_end=subscription.current_period_end) - return False - - return True -``` - -#### Task 1.2: Add subscription validation dependency ✅ -**File**: `apps/cloud/src/basic_memory_cloud/deps.py` - -- [x] Import necessary types at top of file (if not already present) -- [x] Add `authorized_user_profile()` async function -- [x] Implement hybrid JWT validation (WorkOS first, AuthKit fallback) -- [x] Add subscription check using `subscription_service.check_subscription()` -- [x] Raise `HTTPException(403)` with structured error detail if no active subscription -- [x] Look up and return `UserProfile` after validation -- [x] Add `AuthorizedUserProfileDep` type annotation -- [x] Use `settings.subscription_url` from config (env var) -- [x] Run `just typecheck` to verify types - -**Expected code**: -```python -async def get_authorized_cli_user_profile( - credentials: Annotated[HTTPAuthorizationCredentials, Depends(security)], - session: DatabaseSessionDep, - user_profile_repo: UserProfileRepositoryDep, - subscription_service: SubscriptionServiceDep, -) -> UserProfile: - """ - Hybrid authentication with subscription validation for CLI access. - - Validates JWT (WorkOS or AuthKit) and checks for active subscription. - Returns UserProfile if both checks pass. - - Raises: - HTTPException(401): Invalid JWT token - HTTPException(403): No active subscription - """ - # Try WorkOS JWT first (faster validation path) - try: - user_context = await validate_workos_jwt(credentials.credentials) - except HTTPException: - # Fall back to AuthKit JWT validation - try: - user_context = await validate_authkit_jwt(credentials.credentials) - except HTTPException as e: - raise HTTPException( - status_code=401, - detail="Invalid JWT token. Authentication required.", - ) from e - - # Check subscription status - has_subscription = await subscription_service.check_user_has_active_subscription( - session, user_context.workos_user_id - ) - - if not has_subscription: - logger.warning( - "CLI access denied: no active subscription", - workos_user_id=user_context.workos_user_id, - ) - raise HTTPException( - status_code=403, - detail={ - "error": "subscription_required", - "message": "Active subscription required for CLI access", - "subscribe_url": "https://basicmemory.com/subscribe" - } - ) - - # Look up and return user profile - user_profile = await user_profile_repo.get_user_profile_by_workos_user_id( - session, user_context.workos_user_id - ) - if not user_profile: - logger.error( - "User profile not found after successful auth", - workos_user_id=user_context.workos_user_id, - ) - raise HTTPException(401, detail="User profile not found") - - logger.info( - "CLI access granted", - workos_user_id=user_context.workos_user_id, - user_profile_id=str(user_profile.id), - ) - return user_profile - - -AuthorizedCLIUserProfileDep = Annotated[UserProfile, Depends(get_authorized_cli_user_profile)] -``` - -#### Task 1.3: Protect tenant mount endpoints ✅ -**File**: `apps/cloud/src/basic_memory_cloud/api/tenant_mount.py` - -- [x] Update import: add `AuthorizedUserProfileDep` from `..deps` -- [x] Replace `user_profile: CurrentUserProfileHybridJwtDep` with `user_profile: AuthorizedUserProfileDep` in: - - [x] `get_tenant_mount_info()` (line ~23) - - [x] `create_tenant_mount_credentials()` (line ~88) - - [x] `revoke_tenant_mount_credentials()` (line ~244) - - [x] `list_tenant_mount_credentials()` (line ~326) -- [x] Verify no other code changes needed (parameter name and usage stays the same) -- [x] Run `just typecheck` to verify types - -#### Task 1.4: Protect proxy endpoints ✅ -**File**: `apps/cloud/src/basic_memory_cloud/api/proxy.py` - -- [x] Update import: add `AuthorizedUserProfileDep` from `..deps` -- [x] Replace `user_profile: CurrentUserProfileHybridJwtDep` with `user_profile: AuthorizedUserProfileDep` in: - - [x] `check_tenant_health()` (line ~21) - - [x] `proxy_to_tenant()` (line ~63) -- [x] Verify no other code changes needed (parameter name and usage stays the same) -- [x] Run `just typecheck` to verify types - -**Why Keep /proxy Architecture:** - -The proxy layer is valuable because it: -1. **Centralizes authorization** - Single place for JWT + subscription validation (closes both CLI and MCP auth gaps) -2. **Handles tenant routing** - Maps tenant_id → fly_app_name without exposing infrastructure details -3. **Abstracts infrastructure** - MCP and CLI don't need to know about Fly.io naming conventions -4. **Enables features** - Can add rate limiting, caching, request logging, etc. at proxy layer -5. **Supports both flows** - CLI tools and MCP tools both use /proxy endpoints - -The extra HTTP hop is minimal (< 10ms) and worth it for architectural benefits. - -**Performance Note:** Cloud app has Redis available - can cache subscription status to reduce database queries if needed. Initial implementation uses direct database query (simple, acceptable performance ~5-10ms). - -#### Task 1.5: Add unit tests for subscription service -**File**: `apps/cloud/tests/services/test_subscription_service.py` (create if doesn't exist) - -- [ ] Create test file if it doesn't exist -- [ ] Add test: `test_check_user_has_active_subscription_returns_true_for_active()` - - Create user with active subscription - - Call `check_user_has_active_subscription()` - - Assert returns `True` -- [ ] Add test: `test_check_user_has_active_subscription_returns_false_for_pending()` - - Create user with pending subscription - - Assert returns `False` -- [ ] Add test: `test_check_user_has_active_subscription_returns_false_for_cancelled()` - - Create user with cancelled subscription - - Assert returns `False` -- [ ] Add test: `test_check_user_has_active_subscription_returns_false_for_no_subscription()` - - Create user without subscription - - Assert returns `False` -- [ ] Run `just test` to verify tests pass - -#### Task 1.6: Add integration tests for dependency -**File**: `apps/cloud/tests/test_deps.py` (create if doesn't exist) - -- [ ] Create test file if it doesn't exist -- [ ] Add fixtures for mocking JWT credentials -- [ ] Add test: `test_authorized_cli_user_profile_with_active_subscription()` - - Mock valid JWT + active subscription - - Call dependency - - Assert returns UserProfile -- [ ] Add test: `test_authorized_cli_user_profile_without_subscription_raises_403()` - - Mock valid JWT + no subscription - - Assert raises HTTPException(403) with correct error detail -- [ ] Add test: `test_authorized_cli_user_profile_with_inactive_subscription_raises_403()` - - Mock valid JWT + cancelled subscription - - Assert raises HTTPException(403) -- [ ] Add test: `test_authorized_cli_user_profile_with_invalid_jwt_raises_401()` - - Mock invalid JWT - - Assert raises HTTPException(401) -- [ ] Run `just test` to verify tests pass - -#### Task 1.7: Deploy and verify cloud service -- [ ] Run `just check` to verify all quality checks pass -- [ ] Commit changes with message: "feat: add subscription validation to CLI endpoints" -- [ ] Deploy to preview environment: `flyctl deploy --config apps/cloud/fly.toml` -- [ ] Test manually: - - [ ] Call `/tenant/mount/info` with valid JWT but no subscription → expect 403 - - [ ] Call `/tenant/mount/info` with valid JWT and active subscription → expect 200 - - [ ] Verify error response structure matches spec - -### Phase 2: CLI (basic-memory) - -#### Task 2.1: Review and understand CLI authentication flow -**Files**: `src/basic_memory/cli/commands/cloud/` - -- [ ] Read `core_commands.py` to understand current login flow -- [ ] Read `api_client.py` to understand current error handling -- [ ] Identify where 403 errors should be caught -- [ ] Identify what error messages should be displayed -- [ ] Document current behavior in spec if needed - -#### Task 2.2: Update API client error handling -**File**: `src/basic_memory/cli/commands/cloud/api_client.py` - -- [ ] Add custom exception class `SubscriptionRequiredError` (or similar) -- [ ] Update HTTP error handling to parse 403 responses -- [ ] Extract `error`, `message`, and `subscribe_url` from error detail -- [ ] Raise specific exception for subscription_required errors -- [ ] Run `just typecheck` in basic-memory repo to verify types - -#### Task 2.3: Update CLI login command error handling -**File**: `src/basic_memory/cli/commands/cloud/core_commands.py` - -- [ ] Import the subscription error exception -- [ ] Wrap login flow with try/except for subscription errors -- [ ] Display user-friendly error message with rich console -- [ ] Show subscribe URL prominently -- [ ] Provide actionable next steps -- [ ] Run `just typecheck` to verify types - -**Expected error handling**: -```python -try: - # Existing login logic - success = await auth.login() - if success: - # Test access to protected endpoint - await api_client.test_connection() -except SubscriptionRequiredError as e: - console.print("\n[red]✗ Subscription Required[/red]\n") - console.print(f"[yellow]{e.message}[/yellow]\n") - console.print(f"Subscribe at: [blue underline]{e.subscribe_url}[/blue underline]\n") - console.print("[dim]Once you have an active subscription, run [bold]bm cloud login[/bold] again.[/dim]") - raise typer.Exit(1) -``` - -#### Task 2.4: Update CLI tests -**File**: `tests/cli/test_cloud_commands.py` - -- [ ] Add test: `test_login_without_subscription_shows_error()` - - Mock 403 subscription_required response - - Call login command - - Assert error message displayed - - Assert subscribe URL shown -- [ ] Add test: `test_login_with_subscription_succeeds()` - - Mock successful authentication + subscription check - - Call login command - - Assert success message -- [ ] Run `just test` to verify tests pass - -#### Task 2.5: Update CLI documentation -**File**: `docs/cloud-cli.md` (in basic-memory-docs repo) - -- [ ] Add "Prerequisites" section if not present -- [ ] Document subscription requirement -- [ ] Add "Troubleshooting" section -- [ ] Document "Subscription Required" error -- [ ] Provide subscribe URL -- [ ] Add FAQ entry about subscription errors -- [ ] Build docs locally to verify formatting - -### Phase 3: End-to-End Testing - -#### Task 3.1: Create test user accounts -**Prerequisites**: Access to WorkOS admin and database - -- [ ] Create test user WITHOUT subscription: - - [ ] Sign up via WorkOS AuthKit - - [ ] Get workos_user_id from database - - [ ] Verify no subscription record exists - - [ ] Save credentials for testing -- [ ] Create test user WITH active subscription: - - [ ] Sign up via WorkOS AuthKit - - [ ] Create subscription via Polar or dev endpoint - - [ ] Verify subscription.status = "active" in database - - [ ] Save credentials for testing - -#### Task 3.2: Manual testing - User without subscription -**Environment**: Preview/staging deployment - -- [ ] Run `bm cloud login` with no-subscription user -- [ ] Verify: Login shows "Subscription Required" error -- [ ] Verify: Subscribe URL is displayed -- [ ] Verify: Cannot run `bm cloud setup` -- [ ] Verify: Cannot call `/tenant/mount/info` directly via curl -- [ ] Document any issues found - -#### Task 3.3: Manual testing - User with active subscription -**Environment**: Preview/staging deployment - -- [ ] Run `bm cloud login` with active-subscription user -- [ ] Verify: Login succeeds without errors -- [ ] Verify: Can run `bm cloud setup` -- [ ] Verify: Can call `/tenant/mount/info` successfully -- [ ] Verify: Can call `/proxy/*` endpoints successfully -- [ ] Document any issues found - -#### Task 3.4: Test subscription state transitions -**Environment**: Preview/staging deployment + database access - -- [ ] Start with active subscription user -- [ ] Verify: All operations work -- [ ] Update subscription.status to "cancelled" in database -- [ ] Verify: Login now shows "Subscription Required" error -- [ ] Verify: Existing tokens are rejected with 403 -- [ ] Update subscription.status back to "active" -- [ ] Verify: Access restored immediately -- [ ] Document any issues found - -#### Task 3.5: Integration test suite -**File**: `apps/cloud/tests/integration/test_cli_subscription_flow.py` (create if doesn't exist) - -- [ ] Create integration test file -- [ ] Add test: `test_cli_flow_without_subscription()` - - Simulate full CLI flow without subscription - - Assert 403 at appropriate points -- [ ] Add test: `test_cli_flow_with_active_subscription()` - - Simulate full CLI flow with active subscription - - Assert all operations succeed -- [ ] Add test: `test_subscription_expiration_blocks_access()` - - Start with active subscription - - Change status to cancelled - - Assert access denied -- [ ] Run tests in CI/CD pipeline -- [ ] Document test coverage - -#### Task 3.6: Load/performance testing (optional) -**Environment**: Staging environment - -- [ ] Test subscription check performance under load -- [ ] Measure latency added by subscription check -- [ ] Verify database query performance -- [ ] Document any performance concerns -- [ ] Optimize if needed - -## Implementation Summary Checklist - -Use this high-level checklist to track overall progress: - -### Phase 1: Cloud Service 🔄 -- [x] Add subscription check method to SubscriptionService -- [x] Add subscription validation dependency to deps.py -- [x] Add subscription_url config (env var) -- [x] Protect tenant mount endpoints (4 endpoints) -- [x] Protect proxy endpoints (2 endpoints) -- [ ] Add unit tests for subscription service -- [ ] Add integration tests for dependency -- [ ] Deploy and verify cloud service - -### Phase 2: CLI Updates 🔄 -- [ ] Review CLI authentication flow -- [ ] Update API client error handling -- [ ] Update CLI login command error handling -- [ ] Add CLI tests -- [ ] Update CLI documentation - -### Phase 3: End-to-End Testing 🧪 -- [ ] Create test user accounts -- [ ] Manual testing - user without subscription -- [ ] Manual testing - user with active subscription -- [ ] Test subscription state transitions -- [ ] Integration test suite -- [ ] Load/performance testing (optional) - -## Questions to Resolve - -### Resolved ✅ - -1. **Admin Access** - - ✅ **Decision**: Admin users bypass subscription check - - **Rationale**: Admin endpoints already use `AdminUserHybridDep`, which is separate from CLI user endpoints - - **Implementation**: No changes needed to admin endpoints - -2. **Subscription Check Implementation** - - ✅ **Decision**: Use Option A (Database Check) - - **Rationale**: Simpler, faster to implement, works with existing infrastructure - - **Implementation**: Single JOIN query via `get_subscription_by_workos_user_id()` - -3. **Dependency Return Type** - - ✅ **Decision**: Return `UserProfile` (not `UserContext`) - - **Rationale**: Drop-in compatibility with existing endpoints, no refactoring needed - - **Implementation**: `AuthorizedCLIUserProfileDep` returns `UserProfile` - -### To Be Resolved ⏳ - -1. **Subscription Check Frequency** - - **Options**: - - Check on every API call (slower, more secure) ✅ **RECOMMENDED** - - Cache subscription status (faster, risk of stale data) - - Check only on login/setup (fast, but allows expired subscriptions temporarily) - - **Recommendation**: Check on every call via dependency injection (simple, secure, acceptable performance) - - **Impact**: ~5-10ms per request (single indexed JOIN query) - -2. **Grace Period** - - **Options**: - - No grace period - immediate block when status != "active" ✅ **RECOMMENDED** - - 7-day grace period after period_end - - 14-day grace period after period_end - - **Recommendation**: No grace period initially, add later if needed based on customer feedback - - **Implementation**: Check `subscription.status == "active"` only (ignore period_end initially) - -3. **Subscription Expiration Handling** - - **Question**: Should we check `current_period_end < now()` in addition to `status == "active"`? - - **Options**: - - Only check status field (rely on Polar webhooks to update status) ✅ **RECOMMENDED** - - Check both status and current_period_end (more defensive) - - **Recommendation**: Only check status field, assume Polar webhooks keep it current - - **Risk**: If webhooks fail, expired subscriptions might retain access until webhook succeeds - -4. **Subscribe URL** - - **Question**: What's the actual subscription URL? - - **Current**: Spec uses `https://basicmemory.com/subscribe` - - **Action Required**: Verify correct URL before implementation - -5. **Dev Mode / Testing Bypass** - - **Question**: Support bypass for development/testing? - - **Options**: - - Environment variable: `DISABLE_SUBSCRIPTION_CHECK=true` - - Always enforce (more realistic testing) ✅ **RECOMMENDED** - - **Recommendation**: No bypass - use test users with real subscriptions for realistic testing - - **Implementation**: Create dev endpoint to activate subscriptions for testing - -## Related Specs - -- SPEC-9: Multi-Project Bidirectional Sync Architecture (CLI affected by this change) -- SPEC-8: TigrisFS Integration (Mount endpoints protected) - -## Notes - -- This spec prioritizes security over convenience - better to block unauthorized access than risk revenue loss -- Clear error messages are critical - users should understand why they're blocked and how to resolve it -- Consider adding telemetry to track subscription_required errors for monitoring signup conversion diff --git a/specs/SPEC-14 Cloud Git Versioning & GitHub Backup.md b/specs/SPEC-14 Cloud Git Versioning & GitHub Backup.md deleted file mode 100644 index 60ceadd5..00000000 --- a/specs/SPEC-14 Cloud Git Versioning & GitHub Backup.md +++ /dev/null @@ -1,210 +0,0 @@ ---- -title: 'SPEC-14: Cloud Git Versioning & GitHub Backup' -type: spec -permalink: specs/spec-14-cloud-git-versioning -tags: -- git -- github -- backup -- versioning -- cloud -related: -- specs/spec-9-multi-project-bisync -- specs/spec-9-follow-ups-conflict-sync-and-observability -status: deferred ---- - -# SPEC-14: Cloud Git Versioning & GitHub Backup - -**Status: DEFERRED** - Postponed until multi-user/teams feature development. Using S3 versioning (SPEC-9.1) for v1 instead. - -## Why Deferred - -**Original goals can be met with simpler solutions:** -- Version history → **S3 bucket versioning** (automatic, zero config) -- Offsite backup → **Tigris global replication** (built-in) -- Restore capability → **S3 version restore** (`bm cloud restore --version-id`) -- Collaboration → **Deferred to teams/multi-user feature** (not v1 requirement) - -**Complexity vs value trade-off:** -- Git integration adds: committer service, puller service, webhooks, LFS, merge conflicts -- Risk: Loop detection between Git ↔ rclone bisync ↔ local edits -- S3 versioning gives 80% of value with 5% of complexity - -**When to revisit:** -- Teams/multi-user features (PR-based collaboration workflow) -- User requests for commit messages and branch-based workflows -- Need for fine-grained audit trail beyond S3 object metadata - ---- - -## Original Specification (for reference) - -## Why -Early access users want **transparent version history**, easy **offsite backup**, and a familiar **restore/branching** workflow. Git/GitHub integration would provide: -- Auditable history of every change (who/when/why) -- Branches/PRs for review and collaboration -- Offsite private backup under the user's control -- Escape hatch: users can always `git clone` their knowledge base - -**Note:** These goals are now addressed via S3 versioning (SPEC-9.1) for single-user use case. - -## Goals -- **Transparent**: Users keep using Basic Memory; Git runs behind the scenes. -- **Private**: Push to a **private GitHub repo** that the user owns (or tenant org). -- **Reliable**: No data loss, deterministic mapping of filesystem ↔ Git. -- **Composable**: Plays nicely with SPEC‑9 bisync and upcoming conflict features (SPEC‑9 Follow‑Ups). - -**Non‑Goals (for v1):** -- Fine‑grained per‑file encryption in Git history (can be layered later). -- Large media optimization beyond Git LFS defaults. - -## User Stories -1. *As a user*, I connect my GitHub and choose a private backup repo. -2. *As a user*, every change I make in cloud (or via bisync) is **committed** and **pushed** automatically. -3. *As a user*, I can **restore** a file/folder/project to a prior version. -4. *As a power user*, I can **git pull/push** directly to collaborate outside the app. -5. *As an admin*, I can enforce repo ownership (tenant org) and least‑privilege scopes. - -## Scope -- **In scope:** Full repo backup of `/app/data/` (all projects) with optional selective subpaths. -- **Out of scope (v1):** Partial shallow mirrors; encrypted Git; cross‑provider SCM (GitLab/Bitbucket). - -## Architecture -### Topology -- **Authoritative working tree**: `/app/data/` (bucket mount) remains the source of truth (SPEC‑9). -- **Bare repo** lives alongside: `/app/git/${tenant}/knowledge.git` (server‑side). -- **Mirror remote**: `github.com//.git` (private). - -```mermaid -flowchart LR - A[/Users & Agents/] -->|writes/edits| B[/app/data/] - B -->|file events| C[Committer Service] - C -->|git commit| D[(Bare Repo)] - D -->|push| E[(GitHub Private Repo)] - E -->|webhook (push)| F[Puller Service] - F -->|git pull/merge| D - D -->|checkout/merge| B -``` - -### Services -- **Committer Service** (daemon): - - Watches `/app/data/` for changes (inotify/poll) - - Batches changes (debounce e.g. 2–5s) - - Writes `.bmmeta` (if present) into commit message trailer (see Follow‑Ups) - - `git add -A && git commit -m "chore(sync): - -BM-Meta: "` - - Periodic `git push` to GitHub mirror (configurable interval) -- **Puller Service** (webhook target): - - Receives GitHub webhook (push) → `git fetch` - - **Fast‑forward** merges to `main` only; reject non‑FF unless policy allows - - Applies changes back to `/app/data/` via clean checkout - - Emits sync events for Basic Memory indexers - -### Auth & Security -- **GitHub App** (recommended): minimal scopes: `contents:read/write`, `metadata:read`, webhook. -- Tenant‑scoped installation; repo created in user account or tenant org. -- Tokens stored in KMS/secret manager; rotated automatically. -- Optional policy: allow only **FF merges** on `main`; non‑FF requires PR. - -### Repo Layout -- **Monorepo** (default): one repo per tenant mirrors `/app/data/` with subfolders per project. -- Optional multi‑repo mode (later): one repo per project. - -### File Handling -- Honor `.gitignore` generated from `.bmignore.rclone` + BM defaults (cache, temp, state). -- **Git LFS** for large binaries (images, media) — auto track by extension/size threshold. -- Normalize newline + Unicode (aligns with Follow‑Ups). - -### Conflict Model -- **Primary concurrency**: SPEC‑9 Follow‑Ups (`.bmmeta`, conflict copies) stays the first line of defense. -- **Git merges** are a **secondary** mechanism: - - Server only auto‑merges **text** conflicts when trivial (FF or clean 3‑way). - - Otherwise, create `name (conflict from , ).md` and surface via events. - -### Data Flow vs Bisync -- Bisync (rclone) continues between local sync dir ↔ bucket. -- Git sits **cloud‑side** between bucket and GitHub. -- On **pull** from GitHub → files written to `/app/data/` → picked up by indexers & eventually by bisync back to users. - -## CLI & UX -New commands (cloud mode): -- `bm cloud git connect` — Launch GitHub App installation; create private repo; store installation id. -- `bm cloud git status` — Show connected repo, last push time, last webhook delivery, pending commits. -- `bm cloud git push` — Manual push (rarely needed). -- `bm cloud git pull` — Manual pull/FF (admin only by default). -- `bm cloud snapshot -m "message"` — Create a tagged point‑in‑time snapshot (git tag). -- `bm restore --to ` — Restore file/folder/project to prior version. - -Settings: -- `bm config set git.autoPushInterval=5s` -- `bm config set git.lfs.sizeThreshold=10MB` -- `bm config set git.allowNonFF=false` - -## Migration & Backfill -- On connect, if repo empty: initial commit of entire `/app/data/`. -- If repo has content: require **one‑time import** path (clone to staging, reconcile, choose direction). - -## Edge Cases -- Massive deletes: gated by SPEC‑9 `max_delete` **and** Git pre‑push hook checks. -- Case changes and rename detection: rely on git rename heuristics + Follow‑Ups move hints. -- Secrets: default ignore common secret patterns; allow custom deny list. - -## Telemetry & Observability -- Emit `git_commit`, `git_push`, `git_pull`, `git_conflict` events with correlation IDs. -- `bm sync --report` extended with Git stats (commit count, delta bytes, push latency). - -## Phased Plan -### Phase 0 — Prototype (1 sprint) -- Server: bare repo init + simple committer (batch every 10s) + manual GitHub token. -- CLI: `bm cloud git connect --token ` (dev‑only) -- Success: edits in `/app/data/` appear in GitHub within 30s. - -### Phase 1 — GitHub App & Webhooks (1–2 sprints) -- Switch to GitHub App installs; create private repo; store installation id. -- Committer hardened (debounce 2–5s, backoff, retries). -- Puller service with webhook → FF merge → checkout to `/app/data/`. -- LFS auto‑track + `.gitignore` generation. -- CLI surfaces status + logs. - -### Phase 2 — Restore & Snapshots (1 sprint) -- `bm restore` for file/folder/project with dry‑run. -- `bm cloud snapshot` tags + list/inspect. -- Policy: PR‑only non‑FF, admin override. - -### Phase 3 — Selective & Multi‑Repo (nice‑to‑have) -- Include/exclude projects; optional per‑project repos. -- Advanced policies (branch protections, required reviews). - -## Acceptance Criteria -- Changes to `/app/data/` are committed and pushed automatically within configurable interval (default ≤5s). -- GitHub webhook pull results in updated files in `/app/data/` (FF‑only by default). -- LFS configured and functioning; large files don't bloat history. -- `bm cloud git status` shows connected repo and last push/pull times. -- `bm restore` restores a file/folder to a prior commit with a clear audit trail. -- End‑to‑end works alongside SPEC‑9 bisync without loops or data loss. - -## Risks & Mitigations -- **Loop risk (Git ↔ Bisync)**: Writes to `/app/data/` → bisync → local → user edits → back again. *Mitigation*: Debounce, commit squashing, idempotent `.bmmeta` versioning, and watch exclusion windows during pull. -- **Repo bloat**: Lots of binary churn. *Mitigation*: default LFS, size threshold, optional media‑only repo later. -- **Security**: Token leakage. *Mitigation*: GitHub App with short‑lived tokens, KMS storage, scoped permissions. -- **Merge complexity**: Non‑trivial conflicts. *Mitigation*: prefer FF; otherwise conflict copies + events; require PR for non‑FF. - -## Open Questions -- Do we default to **monorepo** per tenant, or offer project‑per‑repo at connect time? -- Should `restore` write to a branch and open a PR, or directly modify `main`? -- How do we expose Git history in UI (timeline view) without users dropping to CLI? - -## Appendix: Sample Config -```json -{ - "git": { - "enabled": true, - "repo": "https://github.com//.git", - "autoPushInterval": "5s", - "allowNonFF": false, - "lfs": { "sizeThreshold": 10485760 } - } -} -``` diff --git a/specs/SPEC-14- Cloud Git Versioning & GitHub Backup.md b/specs/SPEC-14- Cloud Git Versioning & GitHub Backup.md deleted file mode 100644 index 60ceadd5..00000000 --- a/specs/SPEC-14- Cloud Git Versioning & GitHub Backup.md +++ /dev/null @@ -1,210 +0,0 @@ ---- -title: 'SPEC-14: Cloud Git Versioning & GitHub Backup' -type: spec -permalink: specs/spec-14-cloud-git-versioning -tags: -- git -- github -- backup -- versioning -- cloud -related: -- specs/spec-9-multi-project-bisync -- specs/spec-9-follow-ups-conflict-sync-and-observability -status: deferred ---- - -# SPEC-14: Cloud Git Versioning & GitHub Backup - -**Status: DEFERRED** - Postponed until multi-user/teams feature development. Using S3 versioning (SPEC-9.1) for v1 instead. - -## Why Deferred - -**Original goals can be met with simpler solutions:** -- Version history → **S3 bucket versioning** (automatic, zero config) -- Offsite backup → **Tigris global replication** (built-in) -- Restore capability → **S3 version restore** (`bm cloud restore --version-id`) -- Collaboration → **Deferred to teams/multi-user feature** (not v1 requirement) - -**Complexity vs value trade-off:** -- Git integration adds: committer service, puller service, webhooks, LFS, merge conflicts -- Risk: Loop detection between Git ↔ rclone bisync ↔ local edits -- S3 versioning gives 80% of value with 5% of complexity - -**When to revisit:** -- Teams/multi-user features (PR-based collaboration workflow) -- User requests for commit messages and branch-based workflows -- Need for fine-grained audit trail beyond S3 object metadata - ---- - -## Original Specification (for reference) - -## Why -Early access users want **transparent version history**, easy **offsite backup**, and a familiar **restore/branching** workflow. Git/GitHub integration would provide: -- Auditable history of every change (who/when/why) -- Branches/PRs for review and collaboration -- Offsite private backup under the user's control -- Escape hatch: users can always `git clone` their knowledge base - -**Note:** These goals are now addressed via S3 versioning (SPEC-9.1) for single-user use case. - -## Goals -- **Transparent**: Users keep using Basic Memory; Git runs behind the scenes. -- **Private**: Push to a **private GitHub repo** that the user owns (or tenant org). -- **Reliable**: No data loss, deterministic mapping of filesystem ↔ Git. -- **Composable**: Plays nicely with SPEC‑9 bisync and upcoming conflict features (SPEC‑9 Follow‑Ups). - -**Non‑Goals (for v1):** -- Fine‑grained per‑file encryption in Git history (can be layered later). -- Large media optimization beyond Git LFS defaults. - -## User Stories -1. *As a user*, I connect my GitHub and choose a private backup repo. -2. *As a user*, every change I make in cloud (or via bisync) is **committed** and **pushed** automatically. -3. *As a user*, I can **restore** a file/folder/project to a prior version. -4. *As a power user*, I can **git pull/push** directly to collaborate outside the app. -5. *As an admin*, I can enforce repo ownership (tenant org) and least‑privilege scopes. - -## Scope -- **In scope:** Full repo backup of `/app/data/` (all projects) with optional selective subpaths. -- **Out of scope (v1):** Partial shallow mirrors; encrypted Git; cross‑provider SCM (GitLab/Bitbucket). - -## Architecture -### Topology -- **Authoritative working tree**: `/app/data/` (bucket mount) remains the source of truth (SPEC‑9). -- **Bare repo** lives alongside: `/app/git/${tenant}/knowledge.git` (server‑side). -- **Mirror remote**: `github.com//.git` (private). - -```mermaid -flowchart LR - A[/Users & Agents/] -->|writes/edits| B[/app/data/] - B -->|file events| C[Committer Service] - C -->|git commit| D[(Bare Repo)] - D -->|push| E[(GitHub Private Repo)] - E -->|webhook (push)| F[Puller Service] - F -->|git pull/merge| D - D -->|checkout/merge| B -``` - -### Services -- **Committer Service** (daemon): - - Watches `/app/data/` for changes (inotify/poll) - - Batches changes (debounce e.g. 2–5s) - - Writes `.bmmeta` (if present) into commit message trailer (see Follow‑Ups) - - `git add -A && git commit -m "chore(sync): - -BM-Meta: "` - - Periodic `git push` to GitHub mirror (configurable interval) -- **Puller Service** (webhook target): - - Receives GitHub webhook (push) → `git fetch` - - **Fast‑forward** merges to `main` only; reject non‑FF unless policy allows - - Applies changes back to `/app/data/` via clean checkout - - Emits sync events for Basic Memory indexers - -### Auth & Security -- **GitHub App** (recommended): minimal scopes: `contents:read/write`, `metadata:read`, webhook. -- Tenant‑scoped installation; repo created in user account or tenant org. -- Tokens stored in KMS/secret manager; rotated automatically. -- Optional policy: allow only **FF merges** on `main`; non‑FF requires PR. - -### Repo Layout -- **Monorepo** (default): one repo per tenant mirrors `/app/data/` with subfolders per project. -- Optional multi‑repo mode (later): one repo per project. - -### File Handling -- Honor `.gitignore` generated from `.bmignore.rclone` + BM defaults (cache, temp, state). -- **Git LFS** for large binaries (images, media) — auto track by extension/size threshold. -- Normalize newline + Unicode (aligns with Follow‑Ups). - -### Conflict Model -- **Primary concurrency**: SPEC‑9 Follow‑Ups (`.bmmeta`, conflict copies) stays the first line of defense. -- **Git merges** are a **secondary** mechanism: - - Server only auto‑merges **text** conflicts when trivial (FF or clean 3‑way). - - Otherwise, create `name (conflict from , ).md` and surface via events. - -### Data Flow vs Bisync -- Bisync (rclone) continues between local sync dir ↔ bucket. -- Git sits **cloud‑side** between bucket and GitHub. -- On **pull** from GitHub → files written to `/app/data/` → picked up by indexers & eventually by bisync back to users. - -## CLI & UX -New commands (cloud mode): -- `bm cloud git connect` — Launch GitHub App installation; create private repo; store installation id. -- `bm cloud git status` — Show connected repo, last push time, last webhook delivery, pending commits. -- `bm cloud git push` — Manual push (rarely needed). -- `bm cloud git pull` — Manual pull/FF (admin only by default). -- `bm cloud snapshot -m "message"` — Create a tagged point‑in‑time snapshot (git tag). -- `bm restore --to ` — Restore file/folder/project to prior version. - -Settings: -- `bm config set git.autoPushInterval=5s` -- `bm config set git.lfs.sizeThreshold=10MB` -- `bm config set git.allowNonFF=false` - -## Migration & Backfill -- On connect, if repo empty: initial commit of entire `/app/data/`. -- If repo has content: require **one‑time import** path (clone to staging, reconcile, choose direction). - -## Edge Cases -- Massive deletes: gated by SPEC‑9 `max_delete` **and** Git pre‑push hook checks. -- Case changes and rename detection: rely on git rename heuristics + Follow‑Ups move hints. -- Secrets: default ignore common secret patterns; allow custom deny list. - -## Telemetry & Observability -- Emit `git_commit`, `git_push`, `git_pull`, `git_conflict` events with correlation IDs. -- `bm sync --report` extended with Git stats (commit count, delta bytes, push latency). - -## Phased Plan -### Phase 0 — Prototype (1 sprint) -- Server: bare repo init + simple committer (batch every 10s) + manual GitHub token. -- CLI: `bm cloud git connect --token ` (dev‑only) -- Success: edits in `/app/data/` appear in GitHub within 30s. - -### Phase 1 — GitHub App & Webhooks (1–2 sprints) -- Switch to GitHub App installs; create private repo; store installation id. -- Committer hardened (debounce 2–5s, backoff, retries). -- Puller service with webhook → FF merge → checkout to `/app/data/`. -- LFS auto‑track + `.gitignore` generation. -- CLI surfaces status + logs. - -### Phase 2 — Restore & Snapshots (1 sprint) -- `bm restore` for file/folder/project with dry‑run. -- `bm cloud snapshot` tags + list/inspect. -- Policy: PR‑only non‑FF, admin override. - -### Phase 3 — Selective & Multi‑Repo (nice‑to‑have) -- Include/exclude projects; optional per‑project repos. -- Advanced policies (branch protections, required reviews). - -## Acceptance Criteria -- Changes to `/app/data/` are committed and pushed automatically within configurable interval (default ≤5s). -- GitHub webhook pull results in updated files in `/app/data/` (FF‑only by default). -- LFS configured and functioning; large files don't bloat history. -- `bm cloud git status` shows connected repo and last push/pull times. -- `bm restore` restores a file/folder to a prior commit with a clear audit trail. -- End‑to‑end works alongside SPEC‑9 bisync without loops or data loss. - -## Risks & Mitigations -- **Loop risk (Git ↔ Bisync)**: Writes to `/app/data/` → bisync → local → user edits → back again. *Mitigation*: Debounce, commit squashing, idempotent `.bmmeta` versioning, and watch exclusion windows during pull. -- **Repo bloat**: Lots of binary churn. *Mitigation*: default LFS, size threshold, optional media‑only repo later. -- **Security**: Token leakage. *Mitigation*: GitHub App with short‑lived tokens, KMS storage, scoped permissions. -- **Merge complexity**: Non‑trivial conflicts. *Mitigation*: prefer FF; otherwise conflict copies + events; require PR for non‑FF. - -## Open Questions -- Do we default to **monorepo** per tenant, or offer project‑per‑repo at connect time? -- Should `restore` write to a branch and open a PR, or directly modify `main`? -- How do we expose Git history in UI (timeline view) without users dropping to CLI? - -## Appendix: Sample Config -```json -{ - "git": { - "enabled": true, - "repo": "https://github.com//.git", - "autoPushInterval": "5s", - "allowNonFF": false, - "lfs": { "sizeThreshold": 10485760 } - } -} -``` diff --git a/specs/SPEC-15 Configuration Persistence via Tigris for Cloud Tenants.md b/specs/SPEC-15 Configuration Persistence via Tigris for Cloud Tenants.md deleted file mode 100644 index e7192ca0..00000000 --- a/specs/SPEC-15 Configuration Persistence via Tigris for Cloud Tenants.md +++ /dev/null @@ -1,273 +0,0 @@ ---- -title: 'SPEC-15: Configuration Persistence via Tigris for Cloud Tenants' -type: spec -permalink: specs/spec-14-config-persistence-tigris -tags: -- persistence -- tigris -- multi-tenant -- infrastructure -- configuration -status: draft ---- - -# SPEC-15: Configuration Persistence via Tigris for Cloud Tenants - -## Why - -We need to persist Basic Memory configuration across Fly.io deployments without using persistent volumes or external databases. - -**Current Problems:** -- `~/.basic-memory/config.json` lost on every deployment (project configuration) -- `~/.basic-memory/memory.db` lost on every deployment (search index) -- Persistent volumes break clean deployment workflow -- External databases (Turso) require per-tenant token management - -**The Insight:** -The SQLite database is just an **index cache** of the markdown files. It can be rebuilt in seconds from the source markdown files in Tigris. Only the small `config.json` file needs true persistence. - -**Solution:** -- Store `config.json` in Tigris bucket (persistent, small file) -- Rebuild `memory.db` on startup from markdown files (fast, ephemeral) -- No persistent volumes, no external databases, no token management - -## What - -Store Basic Memory configuration in the Tigris bucket and rebuild the database index on tenant machine startup. - -**Affected Components:** -- `basic-memory/src/basic_memory/config.py` - Add configurable config directory - -**Architecture:** - -```bash -# Tigris Bucket (persistent, mounted at /app/data) -/app/data/ - ├── .basic-memory/ - │ └── config.json # ← Project configuration (persistent, accessed via BASIC_MEMORY_CONFIG_DIR) - └── basic-memory/ # ← Markdown files (persistent, BASIC_MEMORY_HOME) - ├── project1/ - └── project2/ - -# Fly Machine (ephemeral) -/app/.basic-memory/ - └── memory.db # ← Rebuilt on startup (fast local disk) -``` - -## How (High Level) - -### 1. Add Configurable Config Directory to Basic Memory - -Currently `ConfigManager` hardcodes `~/.basic-memory/config.json`. Add environment variable to override: - -```python -# basic-memory/src/basic_memory/config.py - -class ConfigManager: - """Manages Basic Memory configuration.""" - - def __init__(self) -> None: - """Initialize the configuration manager.""" - home = os.getenv("HOME", Path.home()) - if isinstance(home, str): - home = Path(home) - - # Allow override via environment variable - if config_dir := os.getenv("BASIC_MEMORY_CONFIG_DIR"): - self.config_dir = Path(config_dir) - else: - self.config_dir = home / DATA_DIR_NAME - - self.config_file = self.config_dir / CONFIG_FILE_NAME - - # Ensure config directory exists - self.config_dir.mkdir(parents=True, exist_ok=True) -``` - -### 2. Rebuild Database on Startup - -Basic Memory already has the sync functionality. Just ensure it runs on startup: - -```python -# apps/api/src/basic_memory_cloud_api/main.py - -@app.on_event("startup") -async def startup_sync(): - """Rebuild database index from Tigris markdown files.""" - logger.info("Starting database rebuild from Tigris") - - # Initialize file sync (rebuilds index from markdown files) - app_config = ConfigManager().config - await initialize_file_sync(app_config) - - logger.info("Database rebuild complete") -``` - -### 3. Environment Configuration - -```bash -# Machine environment variables -BASIC_MEMORY_CONFIG_DIR=/app/data/.basic-memory # Config read/written directly to Tigris -# memory.db stays in default location: /app/.basic-memory/memory.db (local ephemeral disk) -``` - -## Implementation Task List - -### Phase 1: Basic Memory Changes ✅ -- [x] Add `BASIC_MEMORY_CONFIG_DIR` environment variable support to `ConfigManager.__init__()` -- [x] Test config loading from custom directory -- [x] Update tests to verify custom config dir works - -### Phase 2: Tigris Bucket Structure ✅ -- [x] Ensure `.basic-memory/` directory exists in Tigris bucket on tenant creation - - ✅ ConfigManager auto-creates on first run, no explicit provisioning needed -- [x] Initialize `config.json` in Tigris on first tenant deployment - - ✅ ConfigManager creates config.json automatically in BASIC_MEMORY_CONFIG_DIR -- [x] Verify TigrisFS handles hidden directories correctly - - ✅ TigrisFS supports hidden directories (verified in SPEC-8) - -### Phase 3: Deployment Integration ✅ -- [x] Set `BASIC_MEMORY_CONFIG_DIR` environment variable in machine deployment - - ✅ Added to BasicMemoryMachineConfigBuilder in fly_schemas.py -- [x] Ensure database rebuild runs on machine startup via initialization sync - - ✅ sync_worker.py runs initialize_file_sync every 30s (already implemented) -- [x] Handle first-time tenant setup (no config exists yet) - - ✅ ConfigManager creates config.json on first initialization -- [ ] Test deployment workflow with config persistence - -### Phase 4: Testing -- [x] Unit tests for config directory override -- [-] Integration test: deploy → write config → redeploy → verify config persists -- [ ] Integration test: deploy → add project → redeploy → verify project in config -- [ ] Performance test: measure db rebuild time on startup - -### Phase 5: Documentation -- [ ] Document config persistence architecture -- [ ] Update deployment runbook -- [ ] Document startup sequence and timing - -## How to Evaluate - -### Success Criteria - -1. **Config Persistence** - - [ ] config.json persists across deployments - - [ ] Projects list maintained across restarts - - [ ] No manual configuration needed after redeploy - -2. **Database Rebuild** - - [ ] memory.db rebuilt on startup in < 30 seconds - - [ ] All entities indexed correctly - - [ ] Search functionality works after rebuild - -3. **Performance** - - [ ] SQLite queries remain fast (local disk) - - [ ] Config reads acceptable (symlink to Tigris) - - [ ] No noticeable performance degradation - -4. **Deployment Workflow** - - [ ] Clean deployments without volumes - - [ ] No new external dependencies - - [ ] No secret management needed - -### Testing Procedure - -1. **Config Persistence Test** - ```bash - # Deploy tenant - POST /tenants → tenant_id - - # Add a project - basic-memory project add "test-project" ~/test - - # Verify config has project - cat /app/data/.basic-memory/config.json - - # Redeploy machine - fly deploy --app basic-memory-{tenant_id} - - # Verify project still exists - basic-memory project list - ``` - -2. **Database Rebuild Test** - ```bash - # Create notes - basic-memory write "Test Note" --content "..." - - # Redeploy (db lost) - fly deploy --app basic-memory-{tenant_id} - - # Wait for startup sync - sleep 10 - - # Verify note is indexed - basic-memory search "Test Note" - ``` - -3. **Performance Benchmark** - ```bash - # Time the startup sync - time basic-memory sync - - # Should be < 30 seconds for typical tenant - ``` - -## Benefits Over Alternatives - -**vs. Persistent Volumes:** -- ✅ Clean deployment workflow -- ✅ No volume migration needed -- ✅ Simpler infrastructure - -**vs. Turso (External Database):** -- ✅ No per-tenant token management -- ✅ No external service dependencies -- ✅ No additional costs -- ✅ Simpler architecture - -**vs. SQLite on FUSE:** -- ✅ Fast local SQLite performance -- ✅ Only slow reads for small config file -- ✅ Database queries remain fast - -## Implementation Assignment - -**Primary Agent:** `python-developer` -- Add `BASIC_MEMORY_CONFIG_DIR` environment variable to ConfigManager -- Update deployment workflow to set environment variable -- Ensure startup sync runs correctly - -**Review Agent:** `system-architect` -- Validate architecture simplicity -- Review performance implications -- Assess startup timing - -## Dependencies - -- **Internal:** TigrisFS must be working and stable -- **Internal:** Basic Memory sync must be reliable -- **Internal:** SPEC-8 (TigrisFS Integration) must be complete - -## Open Questions - -1. Should we add a health check that waits for db rebuild to complete? -2. Do we need to handle very large knowledge bases (>10k entities) differently? -3. Should we add metrics for startup sync duration? - -## References - -- Basic Memory sync: `basic-memory/src/basic_memory/services/initialization.py` -- Config management: `basic-memory/src/basic_memory/config.py` -- TigrisFS integration: SPEC-8 - ---- - -**Status Updates:** - -- 2025-10-08: Pivoted from Turso to Tigris-based config persistence -- 2025-10-08: Phase 1 complete - BASIC_MEMORY_CONFIG_DIR support added (PR #343) -- 2025-10-08: Phases 2-3 complete - Added BASIC_MEMORY_CONFIG_DIR to machine config - - Config now persists to /app/data/.basic-memory/config.json in Tigris bucket - - Database rebuild already working via sync_worker.py - - Ready for deployment testing (Phase 4) diff --git a/specs/SPEC-16 MCP Cloud Service Consolidation.md b/specs/SPEC-16 MCP Cloud Service Consolidation.md deleted file mode 100644 index a61132ad..00000000 --- a/specs/SPEC-16 MCP Cloud Service Consolidation.md +++ /dev/null @@ -1,800 +0,0 @@ ---- -title: 'SPEC-16: MCP Cloud Service Consolidation' -type: spec -permalink: specs/spec-16-mcp-cloud-service-consolidation -tags: -- architecture -- mcp -- cloud -- performance -- deployment -status: in-progress ---- - -## Status Update - -**Phase 0 (Basic Memory Refactor): ✅ COMPLETE** -- basic-memory PR #344: async_client context manager pattern implemented -- All 17 MCP tools updated to use `async with get_client() as client:` -- CLI commands updated to use context manager -- Removed `inject_auth_header()` and `headers.py` (~100 lines deleted) -- Factory pattern enables clean dependency injection -- Tests passing, typecheck clean - -**Phase 0 Integration: ✅ COMPLETE** -- basic-memory-cloud updated to use async-client-context-manager branch -- Implemented `tenant_direct_client_factory()` with proper context manager pattern -- Removed module-level client override hacks -- Removed unnecessary `/proxy` prefix stripping (tools pass relative URLs) -- Typecheck and lint passing with proper noqa hints -- MCP tools confirmed working via inspector (local testing) - -**Phase 1 (Code Consolidation): ✅ COMPLETE** -- MCP server mounted on Cloud FastAPI app at /mcp endpoint -- AuthKitProvider configured with WorkOS settings -- Combined lifespans (Cloud + MCP) working correctly -- JWT context middleware integrated -- All routes and MCP tools functional - -**Phase 2 (Direct Tenant Transport): ✅ COMPLETE** -- TenantDirectTransport implemented with custom httpx transport -- Per-request JWT extraction via FastMCP DI -- Tenant lookup and signed header generation working -- Direct routing to tenant APIs (eliminating HTTP hop) -- Transport tests passing (11/11) - -**Phase 3 (Testing & Validation): ✅ COMPLETE** -- Typecheck and lint passing across all services -- MCP OAuth authentication working in preview environment -- Tenant isolation via signed headers verified -- Fixed BM_TENANT_HEADER_SECRET mismatch between environments -- MCP tools successfully calling tenant APIs in preview - -**Phase 4 (Deployment Configuration): ✅ COMPLETE** -- Updated apps/cloud/fly.template.toml with MCP environment variables -- Added HTTP/2 backend support for better MCP performance -- Added OAuth protected resource health check -- Removed MCP from preview deployment workflow -- Successfully deployed to preview environment (PR #113) -- All services operational at pr-113-basic-memory-cloud.fly.dev - -**Next Steps:** -- Phase 5: Cleanup (remove apps/mcp directory) -- Phase 6: Production rollout and performance measurement - -# SPEC-16: MCP Cloud Service Consolidation - -## Why - -### Original Architecture Constraints (Now Removed) - -The current architecture deploys MCP Gateway and Cloud Service as separate Fly.io apps: - -**Current Flow:** -``` -LLM Client → MCP Gateway (OAuth) → Cloud Proxy (JWT + header signing) → Tenant API (JWT + header validation) - apps/mcp apps/cloud /proxy apps/api -``` - -This separation was originally necessary because: -1. **Stateful SSE requirement** - MCP needed server-sent events with session state for active project tracking -2. **fastmcp.run limitation** - The FastMCP demo helper didn't support worker processes - -### Why These Constraints No Longer Apply - -1. **State externalized** - Project state moved from in-memory to LLM context (external state) -2. **HTTP transport enabled** - Switched from SSE to stateless HTTP for MCP tools -3. **Worker support added** - Converted from `fastmcp.run()` to `uvicorn.run()` with workers - -### Current Problems - -- **Unnecessary HTTP hop** - MCP tools call Cloud /proxy endpoint which calls tenant API -- **Higher latency** - Extra network round trip for every MCP operation -- **Increased costs** - Two separate Fly.io apps instead of one -- **Complex deployment** - Two services to deploy, monitor, and maintain -- **Resource waste** - Separate database connections, HTTP clients, telemetry overhead - -## What - -### Services Affected - -1. **apps/mcp** - MCP Gateway service (to be merged) -2. **apps/cloud** - Cloud service (will receive MCP functionality) -3. **basic-memory** - Update `async_client.py` to use direct calls -4. **Deployment** - Consolidate Fly.io deployment to single app - -### Components Changed - -**Merged:** -- MCP middleware and telemetry into Cloud app -- MCP tools mounted on Cloud FastAPI instance -- ProxyService used directly by MCP tools (not via HTTP) - -**Kept:** -- `/proxy` endpoint (still needed by web UI) -- All existing Cloud routes (provisioning, webhooks, etc.) -- Dual validation in tenant API (JWT + signed headers) - -**Removed:** -- apps/mcp directory -- Separate MCP Fly.io deployment -- HTTP calls from MCP tools to /proxy endpoint - -## How (High Level) - -### 1. Mount FastMCP on Cloud FastAPI App - -```python -# apps/cloud/src/basic_memory_cloud/main.py - -from basic_memory.mcp.server import mcp -from basic_memory_cloud_mcp.middleware import TelemetryMiddleware - -# Configure MCP OAuth -auth_provider = AuthKitProvider( - authkit_domain=settings.authkit_domain, - base_url=settings.authkit_base_url, - required_scopes=[], -) -mcp.auth = auth_provider -mcp.add_middleware(TelemetryMiddleware()) - -# Mount MCP at /mcp endpoint -mcp_app = mcp.http_app(path="/mcp", stateless_http=True) -app.mount("/mcp", mcp_app) - -# Existing Cloud routes stay at root -app.include_router(proxy_router) -app.include_router(provisioning_router) -# ... etc -``` - -### 2. Direct Tenant Transport (No HTTP Hop) - -Instead of calling `/proxy`, MCP tools call tenant APIs directly via custom httpx transport. - -**Important:** No URL prefix stripping needed. The transport receives relative URLs like `/main/resource/notes/my-note` which are correctly routed to tenant APIs. The `/proxy` prefix only exists for web UI requests to the proxy router, not for MCP tools using the custom transport. - -```python -# apps/cloud/src/basic_memory_cloud/transports/tenant_direct.py - -from httpx import AsyncBaseTransport, Request, Response -from fastmcp.server.dependencies import get_http_headers -import jwt - -class TenantDirectTransport(AsyncBaseTransport): - """Direct transport to tenant APIs, bypassing /proxy endpoint.""" - - async def handle_async_request(self, request: Request) -> Response: - # 1. Get JWT from current MCP request (via FastMCP DI) - http_headers = get_http_headers() - auth_header = http_headers.get("authorization") or http_headers.get("Authorization") - token = auth_header.replace("Bearer ", "") - claims = jwt.decode(token, options={"verify_signature": False}) - workos_user_id = claims["sub"] - - # 2. Look up tenant for user - tenant = await tenant_service.get_tenant_by_user_id(workos_user_id) - - # 3. Build tenant app URL with signed headers - fly_app_name = f"{settings.tenant_prefix}-{tenant.id}" - target_url = f"https://{fly_app_name}.fly.dev{request.url.path}" - - headers = dict(request.headers) - signer = create_signer(settings.bm_tenant_header_secret) - headers.update(signer.sign_tenant_headers(tenant.id)) - - # 4. Make direct call to tenant API - response = await self.client.request( - method=request.method, url=target_url, - headers=headers, content=request.content - ) - return response -``` - -Then configure basic-memory's client factory before mounting MCP: - -```python -# apps/cloud/src/basic_memory_cloud/main.py - -from contextlib import asynccontextmanager -from basic_memory.mcp import async_client -from basic_memory_cloud.transports.tenant_direct import TenantDirectTransport - -# Configure factory for basic-memory's async_client -@asynccontextmanager -async def tenant_direct_client_factory(): - """Factory for creating clients with tenant direct transport.""" - client = httpx.AsyncClient( - transport=TenantDirectTransport(), - base_url="http://direct", - ) - try: - yield client - finally: - await client.aclose() - -# Set factory BEFORE importing MCP tools -async_client.set_client_factory(tenant_direct_client_factory) - -# NOW import - tools will use our factory -import basic_memory.mcp.tools -import basic_memory.mcp.prompts -from basic_memory.mcp.server import mcp - -# Mount MCP - tools use direct transport via factory -app.mount("/mcp", mcp_app) -``` - -**Key benefits:** -- Clean dependency injection via factory pattern -- Per-request tenant resolution via FastMCP DI -- Proper resource cleanup (client.aclose() guaranteed) -- Eliminates HTTP hop entirely -- /proxy endpoint remains for web UI - -### 3. Keep /proxy Endpoint for Web UI - -The existing `/proxy` HTTP endpoint remains functional for: -- Web UI requests -- Future external API consumers -- Backward compatibility - -### 4. Security: Maintain Dual Validation - -**Do NOT remove JWT validation from tenant API.** Keep defense in depth: - -```python -# apps/api - Keep both validations -1. JWT validation (from WorkOS token) -2. Signed header validation (from Cloud/MCP) -``` - -This ensures if the Cloud service is compromised, attackers still cannot access tenant APIs without valid JWTs. - -### 5. Deployment Changes - -**Before:** -- `apps/mcp/fly.template.toml` → MCP Gateway deployment -- `apps/cloud/fly.template.toml` → Cloud Service deployment - -**After:** -- Remove `apps/mcp/fly.template.toml` -- Update `apps/cloud/fly.template.toml` to expose port 8000 for both /mcp and /proxy -- Update deployment scripts to deploy single consolidated app - - -## Basic Memory Dependency: Async Client Refactor - -### Problem -The current `basic_memory.mcp.async_client` creates a module-level `client` at import time: -```python -client = create_client() # Runs immediately when module is imported -``` - -This prevents dependency injection - by the time we can override it, tools have already imported it. - -### Solution: Context Manager Pattern with Auth at Client Creation - -Refactor basic-memory to use httpx's context manager pattern instead of module-level client. - -**Key principle:** Authentication happens at client creation time, not per-request. - -```python -# basic_memory/src/basic_memory/mcp/async_client.py -from contextlib import asynccontextmanager -from httpx import AsyncClient, ASGITransport, Timeout - -# Optional factory override for dependency injection -_client_factory = None - -def set_client_factory(factory): - """Override the default client factory (for cloud app, testing, etc).""" - global _client_factory - _client_factory = factory - -@asynccontextmanager -async def get_client(): - """Get an AsyncClient as a context manager. - - Usage: - async with get_client() as client: - response = await client.get(...) - """ - if _client_factory: - # Cloud app: custom transport handles everything - async with _client_factory() as client: - yield client - else: - # Default: create based on config - config = ConfigManager().config - timeout = Timeout(connect=10.0, read=30.0, write=30.0, pool=30.0) - - if config.cloud_mode_enabled: - # CLI cloud mode: inject auth when creating client - from basic_memory.cli.auth import CLIAuth - - auth = CLIAuth( - client_id=config.cloud_client_id, - authkit_domain=config.cloud_domain - ) - token = await auth.get_valid_token() - - if not token: - raise RuntimeError( - "Cloud mode enabled but not authenticated. " - "Run 'basic-memory cloud login' first." - ) - - # Auth header set ONCE at client creation - async with AsyncClient( - base_url=f"{config.cloud_host}/proxy", - headers={"Authorization": f"Bearer {token}"}, - timeout=timeout - ) as client: - yield client - else: - # Local mode: ASGI transport - async with AsyncClient( - transport=ASGITransport(app=fastapi_app), - base_url="http://test", - timeout=timeout - ) as client: - yield client -``` - -**Tool Updates:** -```python -# Before: from basic_memory.mcp.async_client import client -from basic_memory.mcp.async_client import get_client - -async def read_note(...): - # Before: response = await call_get(client, path, ...) - async with get_client() as client: - response = await call_get(client, path, ...) - # ... use response -``` - -**Cloud Usage:** -```python -from contextlib import asynccontextmanager -from basic_memory.mcp import async_client - -@asynccontextmanager -async def tenant_direct_client(): - """Factory for creating clients with tenant direct transport.""" - client = httpx.AsyncClient( - transport=TenantDirectTransport(), - base_url="http://direct", - ) - try: - yield client - finally: - await client.aclose() - -# Before importing MCP tools: -async_client.set_client_factory(tenant_direct_client) - -# Now import - tools will use our factory -import basic_memory.mcp.tools -``` - -### Benefits -- **No module-level state** - client created only when needed -- **Proper cleanup** - context manager ensures `aclose()` is called -- **Easy dependency injection** - factory pattern allows custom clients -- **httpx best practices** - follows official recommendations -- **Works for all modes** - stdio, cloud, testing - -### Architecture Simplification: Auth at Client Creation - -**Key design principle:** Authentication happens when creating the client, not on every request. - -**Three modes, three approaches:** - -1. **Local mode (ASGI)** - - No auth needed - - Direct in-process calls via ASGITransport - -2. **CLI cloud mode (HTTP)** - - Auth token from CLIAuth (stored in ~/.basic-memory/basic-memory-cloud.json) - - Injected as default header when creating AsyncClient - - Single auth check at client creation time - -3. **Cloud app mode (Custom Transport)** - - TenantDirectTransport handles everything - - Extracts JWT from FastMCP context per-request - - No interaction with inject_auth_header() logic - -**What this removes:** -- `src/basic_memory/mcp/tools/headers.py` - entire file deleted -- `inject_auth_header()` calls in all request helpers (call_get, call_post, etc.) -- Per-request header manipulation complexity -- Circular dependency concerns between async_client and auth logic - -**Benefits:** -- Cleaner separation of concerns -- Simpler request helper functions -- Auth happens at the right layer (client creation) -- Cloud app transport is completely independent - -### Refactor Summary - -This refactor achieves: - -**Simplification:** -- Removes ~100 lines of per-request header injection logic -- Deletes entire `headers.py` module -- Auth happens once at client creation, not per-request - -**Decoupling:** -- Cloud app's custom transport is completely independent -- No interaction with basic-memory's auth logic -- Each mode (local, CLI cloud, cloud app) has clean separation - -**Better Design:** -- Follows httpx best practices (context managers) -- Proper resource cleanup (client.aclose() guaranteed) -- Easier testing via factory injection -- No circular import risks - -**Three Distinct Modes:** -1. Local: ASGI transport, no auth -2. CLI cloud: HTTP transport with CLIAuth token injection -3. Cloud app: Custom transport with per-request tenant routing - -### Implementation Plan Summary -1. Create branch `async-client-context-manager` in basic-memory -2. Update `async_client.py` with context manager pattern and CLIAuth integration -3. Remove `inject_auth_header()` from all request helpers -4. Delete `src/basic_memory/mcp/tools/headers.py` -5. Update all MCP tools to use `async with get_client() as client:` -6. Update CLI commands to use context manager and remove manual auth -7. Remove `api_url` config field -8. Update tests -9. Update basic-memory-cloud to use branch: `basic-memory @ git+https://github.com/basicmachines-co/basic-memory.git@async-client-context-manager` - -Detailed breakdown in Phase 0 tasks below. - -### Implementation Notes - -**Potential Issues & Solutions:** - -1. **Circular Import** (async_client imports CLIAuth) - - **Risk:** CLIAuth might import something from async_client - - **Solution:** Use lazy import inside `get_client()` function - - **Already done:** Import is inside the function, not at module level - -2. **Test Fixtures** - - **Risk:** Tests using module-level client will break - - **Solution:** Update fixtures to use factory pattern - - **Example:** - ```python - @pytest.fixture - def mock_client_factory(): - @asynccontextmanager - async def factory(): - async with AsyncClient(...) as client: - yield client - return factory - ``` - -3. **Performance** - - **Risk:** Creating client per tool call might be expensive - - **Reality:** httpx is designed for this pattern, connection pooling at transport level - - **Mitigation:** Monitor performance, can optimize later if needed - -4. **CLI Cloud Commands Edge Cases** - - **Risk:** Token expires mid-operation - - **Solution:** CLIAuth.get_valid_token() already handles refresh - - **Validation:** Test cloud login → use tools → token refresh flow - -5. **Backward Compatibility** - - **Risk:** External code importing `client` directly - - **Solution:** Keep `create_client()` and `client` for one version, deprecate - - **Timeline:** Remove in next major version - -## Implementation Tasks - -### Phase 0: Basic Memory Refactor (Prerequisite) - -#### 0.1 Core Refactor - async_client.py -- [x] Create branch `async-client-context-manager` in basic-memory repo -- [x] Implement `get_client()` context manager -- [x] Implement `set_client_factory()` for dependency injection -- [x] Add CLI cloud mode auth injection (CLIAuth integration) -- [x] Remove `api_url` config field (legacy, unused) -- [x] Keep `create_client()` temporarily for backward compatibility (deprecate later) - -#### 0.2 Simplify Request Helpers - tools/utils.py -- [x] Remove `inject_auth_header()` calls from `call_get()` -- [x] Remove `inject_auth_header()` calls from `call_post()` -- [x] Remove `inject_auth_header()` calls from `call_put()` -- [x] Remove `inject_auth_header()` calls from `call_patch()` -- [x] Remove `inject_auth_header()` calls from `call_delete()` -- [x] Delete `src/basic_memory/mcp/tools/headers.py` entirely -- [x] Update imports in utils.py - -#### 0.3 Update MCP Tools (~16 files) -Convert from `from async_client import client` to `async with get_client() as client:` - -- [x] `tools/write_note.py` (34/34 tests passing) -- [x] `tools/read_note.py` (21/21 tests passing) -- [x] `tools/view_note.py` (12/12 tests passing - no changes needed, delegates to read_note) -- [x] `tools/delete_note.py` (2/2 tests passing) -- [x] `tools/read_content.py` (20/20 tests passing) -- [x] `tools/list_directory.py` (11/11 tests passing) -- [x] `tools/move_note.py` (34/34 tests passing, 90% coverage) -- [x] `tools/search.py` (16/16 tests passing, 96% coverage) -- [x] `tools/recent_activity.py` (4/4 tests passing, 82% coverage) -- [x] `tools/project_management.py` (3 functions: list_memory_projects, create_memory_project, delete_project - typecheck passed) -- [x] `tools/edit_note.py` (17/17 tests passing) -- [x] `tools/canvas.py` (5/5 tests passing) -- [x] `tools/build_context.py` (6/6 tests passing) -- [x] `tools/sync_status.py` (typecheck passed) -- [x] `prompts/continue_conversation.py` (typecheck passed) -- [x] `prompts/search.py` (typecheck passed) -- [x] `resources/project_info.py` (typecheck passed) - -#### 0.4 Update CLI Commands (~3 files) -Remove manual auth header passing, use context manager: - -- [x] `cli/commands/project.py` - removed get_authenticated_headers() calls, use context manager -- [x] `cli/commands/status.py` - use context manager -- [x] `cli/commands/command_utils.py` - use context manager - -#### 0.5 Update Config -- [x] Remove `api_url` field from `BasicMemoryConfig` in config.py -- [x] Update any lingering references/docs (added deprecation notice to v15-docs/cloud-mode-usage.md) - -#### 0.6 Testing -- [-] Update test fixtures to use factory pattern -- [x] Run full test suite in basic-memory -- [x] Verify cloud_mode_enabled works with CLIAuth injection -- [x] Run typecheck and linting - -#### 0.7 Cloud Integration Prep -- [x] Update basic-memory-cloud pyproject.toml to use branch -- [x] Implement factory pattern in cloud app main.py -- [x] Remove `/proxy` prefix stripping logic (not needed - tools pass relative URLs) - -#### 0.8 Phase 0 Validation - -**Before merging async-client-context-manager branch:** - -- [x] All tests pass locally -- [x] Typecheck passes (pyright/mypy) -- [x] Linting passes (ruff) -- [x] Manual test: local mode works (ASGI transport) -- [x] Manual test: cloud login → cloud mode works (HTTP transport with auth) -- [x] No import of `inject_auth_header` anywhere -- [x] `headers.py` file deleted -- [x] `api_url` config removed -- [x] Tool functions properly scoped (client inside async with) -- [ ] CLI commands properly scoped (client inside async with) - -**Integration validation:** -- [x] basic-memory-cloud can import and use factory pattern -- [x] TenantDirectTransport works without touching header injection -- [x] No circular imports or lazy import issues -- [x] MCP tools work via inspector (local testing confirmed) - -### Phase 1: Code Consolidation -- [x] Create feature branch `consolidate-mcp-cloud` -- [x] Update `apps/cloud/src/basic_memory_cloud/config.py`: - - [x] Add `authkit_base_url` field (already has authkit_domain) - - [x] Workers config already exists ✓ -- [x] Update `apps/cloud/src/basic_memory_cloud/telemetry.py`: - - [x] Add `logfire.instrument_mcp()` to existing setup - - [x] Skip complex two-phase setup - use Cloud's simpler approach -- [x] Create `apps/cloud/src/basic_memory_cloud/middleware/jwt_context.py`: - - [x] FastAPI middleware to extract JWT claims from Authorization header - - [x] Add tenant context (workos_user_id) to logfire baggage - - [x] Simpler than FastMCP middleware version -- [x] Update `apps/cloud/src/basic_memory_cloud/main.py`: - - [x] Import FastMCP server from basic-memory - - [x] Configure AuthKitProvider with WorkOS settings - - [x] No FastMCP telemetry middleware needed (using FastAPI middleware instead) - - [x] Create MCP ASGI app: `mcp_app = mcp.http_app(path='/mcp', stateless_http=True)` - - [x] Combine lifespans (Cloud + MCP) using nested async context managers - - [x] Mount MCP: `app.mount("/mcp", mcp_app)` - - [x] Add JWT context middleware to FastAPI app -- [x] Run typecheck - passes ✓ - -### Phase 2: Direct Tenant Transport -- [x] Create `apps/cloud/src/basic_memory_cloud/transports/tenant_direct.py`: - - [x] Implement `TenantDirectTransport(AsyncBaseTransport)` - - [x] Use FastMCP DI (`get_http_headers()`) to extract JWT per-request - - [x] Decode JWT to get `workos_user_id` - - [x] Look up/create tenant via `TenantRepository.get_or_create_tenant_for_workos_user()` - - [x] Build tenant app URL and add signed headers - - [x] Make direct httpx call to tenant API - - [x] No `/proxy` prefix stripping needed (tools pass relative URLs like `/main/resource/...`) -- [x] Update `apps/cloud/src/basic_memory_cloud/main.py`: - - [x] Refactored to use factory pattern instead of module-level override - - [x] Implement `tenant_direct_client_factory()` context manager - - [x] Call `async_client.set_client_factory()` before importing MCP tools - - [x] Clean imports, proper noqa hints for lint -- [x] Basic-memory refactor integrated (PR #344) -- [x] Run typecheck - passes ✓ -- [x] Run lint - passes ✓ - -### Phase 3: Testing & Validation -- [x] Run `just typecheck` in apps/cloud -- [x] Run `just check` in project -- [x] Run `just fix` - all lint errors fixed ✓ -- [x] Write comprehensive transport tests (11 tests passing) ✓ -- [x] Test MCP tools locally with consolidated service (inspector confirmed working) -- [x] Verify OAuth authentication works (requires full deployment) -- [x] Verify tenant isolation via signed headers (requires full deployment) -- [x] Test /proxy endpoint still works for web UI -- [ ] Measure latency before/after consolidation -- [ ] Check telemetry traces span correctly - -### Phase 4: Deployment Configuration -- [x] Update `apps/cloud/fly.template.toml`: - - [x] Merged MCP-specific environment variables (AUTHKIT_BASE_URL, FASTMCP_LOG_LEVEL, BASIC_MEMORY_*) - - [x] Added HTTP/2 backend support (`h2_backend = true`) for better MCP performance - - [x] Added health check for MCP OAuth endpoint (`/.well-known/oauth-protected-resource`) - - [x] Port 8000 already exposed - serves both Cloud routes and /mcp endpoint - - [x] Workers configured (UVICORN_WORKERS = 4) -- [x] Update `.env.example`: - - [x] Consolidated MCP Gateway section into Cloud app section - - [x] Added AUTHKIT_BASE_URL, FASTMCP_LOG_LEVEL, BASIC_MEMORY_HOME - - [x] Added LOG_LEVEL to Development Settings - - [x] Documented that MCP now served at /mcp on Cloud service (port 8000) -- [x] Test deployment to preview environment (PR #113) - - [x] OAuth authentication verified - - [x] MCP tools successfully calling tenant APIs - - [x] Fixed BM_TENANT_HEADER_SECRET synchronization issue - -### Phase 5: Cleanup -- [x] Remove `apps/mcp/` directory entirely -- [x] Remove MCP-specific fly.toml and deployment configs -- [x] Update repository documentation -- [x] Update CLAUDE.md with new architecture -- [-] Archive old MCP deployment configs (if needed) - -### Phase 6: Production Rollout -- [ ] Deploy to development and validate -- [ ] Monitor metrics and logs -- [ ] Deploy to production -- [ ] Verify production functionality -- [ ] Document performance improvements - -## Migration Plan - -### Phase 1: Preparation -1. Create feature branch `consolidate-mcp-cloud` -2. Update basic-memory async_client.py for direct ProxyService calls -3. Update apps/cloud/main.py to mount MCP - -### Phase 2: Testing -1. Local testing with consolidated app -2. Deploy to development environment -3. Run full test suite -4. Performance benchmarking - -### Phase 3: Deployment -1. Deploy to development -2. Validate all functionality -3. Deploy to production -4. Monitor for issues - -### Phase 4: Cleanup -1. Remove apps/mcp directory -2. Update documentation -3. Update deployment scripts -4. Archive old MCP deployment configs - -## Rollback Plan - -If issues arise: -1. Revert feature branch -2. Redeploy separate apps/mcp and apps/cloud services -3. Restore previous fly.toml configurations -4. Document issues encountered - -The well-organized code structure makes splitting back out feasible if future scaling needs diverge. - -## How to Evaluate - -### 1. Functional Testing - -**MCP Tools:** -- [ ] All 17 MCP tools work via consolidated /mcp endpoint -- [x] OAuth authentication validates correctly -- [x] Tenant isolation maintained via signed headers -- [x] Project management tools function correctly - -**Cloud Routes:** -- [x] /proxy endpoint still works for web UI -- [x] /provisioning routes functional -- [x] /webhooks routes functional -- [x] /tenants routes functional - -**API Validation:** -- [x] Tenant API validates both JWT and signed headers -- [x] Unauthorized requests rejected appropriately -- [x] Multi-tenant isolation verified - -### 2. Performance Testing - -**Latency Reduction:** -- [x] Measure MCP tool latency before consolidation -- [x] Measure MCP tool latency after consolidation -- [x] Verify reduction from eliminated HTTP hop (expected: 20-50ms improvement) - -**Resource Usage:** -- [x] Single app uses less total memory than two apps -- [x] Database connection pooling more efficient -- [x] HTTP client overhead reduced - -### 3. Deployment Testing - -**Fly.io Deployment:** -- [x] Single app deploys successfully -- [x] Health checks pass for consolidated service -- [x] No apps/mcp deployment required -- [x] Environment variables configured correctly - -**Local Development:** -- [x] `just setup` works with consolidated architecture -- [x] Local testing shows MCP tools working -- [x] No regression in developer experience - -### 4. Security Validation - -**Defense in Depth:** -- [x] Tenant API still validates JWT tokens -- [x] Tenant API still validates signed headers -- [x] No access possible with only signed headers (JWT required) -- [x] No access possible with only JWT (signed headers required) - -**Authorization:** -- [x] Users can only access their own tenant data -- [x] Cross-tenant requests rejected -- [x] Admin operations require proper authentication - -### 5. Observability - -**Telemetry:** -- [x] OpenTelemetry traces span across MCP → ProxyService → Tenant API -- [x] Logfire shows consolidated traces correctly -- [x] Error tracking and debugging still functional -- [x] Performance metrics accurate - -**Logging:** -- [x] Structured logs show proper context (tenant_id, operation, etc.) -- [x] Error logs contain actionable information -- [x] Log volume reasonable for single app - -## Success Criteria - -1. **Functionality**: All MCP tools and Cloud routes work identically to before -2. **Performance**: Measurable latency reduction (>20ms average) -3. **Cost**: Single Fly.io app instead of two (50% infrastructure reduction) -4. **Security**: Dual validation maintained, no security regression -5. **Deployment**: Simplified deployment process, single app to manage -6. **Observability**: Telemetry and logging work correctly - - - -## Notes - -### Future Considerations - -- **Independent scaling**: If MCP and Cloud need different scaling profiles in future, code organization supports splitting back out -- **Regional deployment**: Consolidated app can still be deployed to multiple regions -- **Edge caching**: Could add edge caching layer in front of consolidated service - -### Dependencies - -- SPEC-9: Signed Header Tenant Information (already implemented) -- SPEC-12: OpenTelemetry Observability (telemetry must work across merged services) - -### Related Work - -- basic-memory v0.13.x: MCP server implementation -- FastMCP documentation: Mounting on existing FastAPI apps -- Fly.io multi-service patterns diff --git a/specs/SPEC-17 Semantic Search with ChromaDB.md b/specs/SPEC-17 Semantic Search with ChromaDB.md deleted file mode 100644 index 4d761bf3..00000000 --- a/specs/SPEC-17 Semantic Search with ChromaDB.md +++ /dev/null @@ -1,1439 +0,0 @@ ---- -title: 'SPEC-17: Semantic Search with ChromaDB' -type: spec -permalink: specs/spec-17-semantic-search-chromadb -tags: -- search -- chromadb -- semantic-search -- vector-database -- postgres-migration ---- - -# SPEC-17: Semantic Search with ChromaDB - -Why ChromaDB for Knowledge Management - -Your users aren't just searching for keywords - they're trying to: -- "Find notes related to this concept" -- "Show me similar ideas" -- "What else did I write about this topic?" - -Example: - # User searches: "AI ethics" - - # FTS5/MeiliSearch finds: - - "AI ethics guidelines" ✅ - - "ethical AI development" ✅ - - "artificial intelligence" ❌ No keyword match - - # ChromaDB finds: - - "AI ethics guidelines" ✅ - - "ethical AI development" ✅ - - "artificial intelligence" ✅ Semantic match! - - "bias in ML models" ✅ Related concept - - "responsible technology" ✅ Similar theme - - "neural network fairness" ✅ Connected idea - -ChromaDB vs MeiliSearch vs Typesense - -| Feature | ChromaDB | MeiliSearch | Typesense | -|------------------|--------------------|--------------------|--------------------| -| Semantic Search | ✅ Excellent | ❌ No | ❌ No | -| Keyword Search | ⚠️ Via metadata | ✅ Excellent | ✅ Excellent | -| Local Deployment | ✅ Embedded mode | ⚠️ Server required | ⚠️ Server required | -| No Server Needed | ✅ YES! | ❌ No | ❌ No | -| Embedding Cost | ~$0.13/1M tokens | None | None | -| Search Speed | 50-200ms | 10-50ms | 10-50ms | -| Best For | Semantic discovery | Exact terms | Exact terms | - -The Killer Feature: Embedded Mode - -ChromaDB has an embedded client that runs in-process - NO SERVER NEEDED! - -# Local (FOSS) - ChromaDB embedded in Python process -import chromadb - -client = chromadb.PersistentClient(path="/path/to/chroma_data") -collection = client.get_or_create_collection("knowledge_base") - -# Add documents -collection.add( - ids=["note1", "note2"], - documents=["AI ethics", "Neural networks"], - metadatas=[{"type": "note"}, {"type": "spec"}] -) - -# Search - NO API calls, runs locally! -results = collection.query( - query_texts=["machine learning"], - n_results=10 -) - - -## Why - -### Current Problem: Database Persistence in Cloud -In cloud deployments, `memory.db` (SQLite) doesn't persist across Docker container restarts. This means: -- Database must be rebuilt on every container restart -- Initial sync takes ~49 seconds for 500 files (after optimization in #352) -- Users experience delays on each deployment - -### Search Architecture Issues -Current SQLite FTS5 implementation creates a **dual-implementation problem** for PostgreSQL migration: -- FTS5 (SQLite) uses `VIRTUAL TABLE` with `MATCH` queries -- PostgreSQL full-text search uses `TSVECTOR` with `@@` operator -- These are fundamentally incompatible architectures -- Would require **2x search code** and **2x tests** to support both - -**Example of incompatibility:** -```python -# SQLite FTS5 -"content_stems MATCH :text" - -# PostgreSQL -"content_vector @@ plainto_tsquery(:text)" -``` - -### Search Quality Limitations -Current keyword-based FTS5 has limitations: -- No semantic understanding (search "AI" doesn't find "machine learning") -- No word relationships (search "neural networks" doesn't find "deep learning") -- Limited typo tolerance -- No relevance ranking beyond keyword matching - -### Strategic Goal: PostgreSQL Migration -Moving to PostgreSQL (Neon) for cloud deployments would: -- ✅ Solve persistence issues (database survives restarts) -- ✅ Enable multi-tenant architecture -- ✅ Better performance for large datasets -- ✅ Support for cloud-native scaling - -**But requires solving the search compatibility problem.** - -## What - -Migrate from SQLite FTS5 to **ChromaDB** for semantic vector search across all deployments. - -**Key insight:** ChromaDB is **database-agnostic** - it works with both SQLite and PostgreSQL, eliminating the dual-implementation problem. - -### Affected Areas -- Search implementation (`src/basic_memory/repository/search_repository.py`) -- Search service (`src/basic_memory/services/search_service.py`) -- Search models (`src/basic_memory/models/search.py`) -- Database initialization (`src/basic_memory/db.py`) -- MCP search tools (`src/basic_memory/mcp/tools/search.py`) -- Dependencies (`pyproject.toml` - add ChromaDB) -- Alembic migrations (FTS5 table removal) -- Documentation - -### What Changes -**Removed:** -- SQLite FTS5 virtual table -- `MATCH` query syntax -- FTS5-specific tokenization and prefix handling -- ~300 lines of FTS5 query preparation code - -**Added:** -- ChromaDB persistent client (embedded mode) -- Vector embedding generation -- Semantic similarity search -- Local embedding model (`sentence-transformers`) -- Collection management for multi-project support - -### What Stays the Same -- Search API interface (MCP tools, REST endpoints) -- Entity/Observation/Relation indexing workflow -- Multi-project isolation -- Search filtering by type, date, metadata -- Pagination and result formatting -- **All SQL queries for exact lookups and metadata filtering** - -## Hybrid Architecture: SQL + ChromaDB - -**Critical Design Decision:** ChromaDB **complements** SQL, it doesn't **replace** it. - -### Why Hybrid? - -ChromaDB is excellent for semantic text search but terrible for exact lookups. SQL is perfect for exact lookups and structured queries. We use both: - -``` -┌─────────────────────────────────────────────────┐ -│ Search Request │ -└─────────────────────────────────────────────────┘ - ▼ - ┌────────────────────────┐ - │ SearchRepository │ - │ (Smart Router) │ - └────────────────────────┘ - ▼ ▼ - ┌───────────┐ ┌──────────────┐ - │ SQL │ │ ChromaDB │ - │ Queries │ │ Semantic │ - └───────────┘ └──────────────┘ - ▼ ▼ - Exact lookups Text search - - Permalink - Semantic similarity - - Pattern match - Related concepts - - Title exact - Typo tolerance - - Metadata filter - Fuzzy matching - - Date ranges -``` - -### When to Use Each - -#### Use SQL For (Fast & Exact) - -**Exact Permalink Lookup:** -```python -# Find by exact permalink - SQL wins -"SELECT * FROM entities WHERE permalink = 'specs/search-feature'" -# ~1ms, perfect for exact matches - -# ChromaDB would be: ~50ms, wasteful -``` - -**Pattern Matching:** -```python -# Find all specs - SQL wins -"SELECT * FROM entities WHERE permalink GLOB 'specs/*'" -# ~5ms, perfect for wildcards - -# ChromaDB doesn't support glob patterns -``` - -**Pure Metadata Queries:** -```python -# Find all meetings tagged "important" - SQL wins -"SELECT * FROM entities - WHERE json_extract(entity_metadata, '$.entity_type') = 'meeting' - AND json_extract(entity_metadata, '$.tags') LIKE '%important%'" -# ~5ms, structured query - -# No text search needed, SQL is faster and simpler -``` - -**Date Filtering:** -```python -# Find recent specs - SQL wins -"SELECT * FROM entities - WHERE entity_type = 'spec' - AND created_at > '2024-01-01' - ORDER BY created_at DESC" -# ~2ms, perfect for structured data -``` - -#### Use ChromaDB For (Semantic & Fuzzy) - -**Semantic Content Search:** -```python -# Find notes about "neural networks" - ChromaDB wins -collection.query(query_texts=["neural networks"]) -# Finds: "machine learning", "deep learning", "AI models" -# ~50-100ms, semantic understanding - -# SQL FTS5 would only find exact keyword matches -``` - -**Text Search + Metadata:** -```python -# Find meeting notes about "project planning" tagged "important" -collection.query( - query_texts=["project planning"], - where={ - "entity_type": "meeting", - "tags": {"$contains": "important"} - } -) -# ~100ms, semantic search with filters -# Finds: "roadmap discussion", "sprint planning", etc. -``` - -**Typo Tolerance:** -```python -# User types "serch feature" (typo) - ChromaDB wins -collection.query(query_texts=["serch feature"]) -# Still finds: "search feature" documents -# ~50-100ms, fuzzy matching - -# SQL would find nothing -``` - -### Performance Comparison - -| Query Type | SQL | ChromaDB | Winner | -|-----------|-----|----------|--------| -| Exact permalink | 1-2ms | 50ms | ✅ SQL | -| Pattern match (specs/*) | 5-10ms | N/A | ✅ SQL | -| Pure metadata filter | 5ms | 50ms | ✅ SQL | -| Semantic text search | ❌ Can't | 50-100ms | ✅ ChromaDB | -| Text + metadata | ❌ Keywords only | 100ms | ✅ ChromaDB | -| Typo tolerance | ❌ Can't | 50ms | ✅ ChromaDB | - -### Metadata/Frontmatter Handling - -**Both systems support full frontmatter filtering!** - -#### SQL Metadata Storage - -```python -# Entities table stores frontmatter as JSON -CREATE TABLE entities ( - id INTEGER PRIMARY KEY, - title TEXT, - permalink TEXT, - file_path TEXT, - entity_type TEXT, - entity_metadata JSON, -- All frontmatter here! - created_at DATETIME, - ... -) - -# Query frontmatter fields -SELECT * FROM entities -WHERE json_extract(entity_metadata, '$.entity_type') = 'meeting' - AND json_extract(entity_metadata, '$.tags') LIKE '%important%' - AND json_extract(entity_metadata, '$.status') = 'completed' -``` - -#### ChromaDB Metadata Storage - -```python -# When indexing, store ALL frontmatter as metadata -class ChromaSearchBackend: - async def index_entity(self, entity: Entity): - """Index with complete frontmatter metadata.""" - - # Extract ALL frontmatter fields - metadata = { - "entity_id": entity.id, - "project_id": entity.project_id, - "permalink": entity.permalink, - "file_path": entity.file_path, - "entity_type": entity.entity_type, - "type": "entity", - # ALL frontmatter tags - "tags": entity.entity_metadata.get("tags", []), - # Custom frontmatter fields - "status": entity.entity_metadata.get("status"), - "priority": entity.entity_metadata.get("priority"), - # Spread any other custom fields - **{k: v for k, v in entity.entity_metadata.items() - if k not in ["tags", "entity_type"]} - } - - self.collection.upsert( - ids=[f"entity_{entity.id}_{entity.project_id}"], - documents=[self._format_document(entity)], - metadatas=[metadata] # Full frontmatter! - ) -``` - -#### ChromaDB Metadata Queries - -ChromaDB supports rich filtering: - -```python -# Simple filter - single field -collection.query( - query_texts=["project planning"], - where={"entity_type": "meeting"} -) - -# Multiple conditions (AND) -collection.query( - query_texts=["architecture decisions"], - where={ - "entity_type": "spec", - "tags": {"$contains": "important"} - } -) - -# Complex filters with operators -collection.query( - query_texts=["machine learning"], - where={ - "$and": [ - {"entity_type": {"$in": ["note", "spec"]}}, - {"tags": {"$contains": "AI"}}, - {"created_at": {"$gt": "2024-01-01"}}, - {"status": "in-progress"} - ] - } -) - -# Multiple tags (all must match) -collection.query( - query_texts=["cloud architecture"], - where={ - "$and": [ - {"tags": {"$contains": "architecture"}}, - {"tags": {"$contains": "cloud"}} - ] - } -) -``` - -### Smart Routing Implementation - -```python -class SearchRepository: - def __init__( - self, - session_maker: async_sessionmaker[AsyncSession], - project_id: int, - chroma_backend: ChromaSearchBackend - ): - self.sql = session_maker # Keep SQL! - self.chroma = chroma_backend - self.project_id = project_id - - async def search( - self, - search_text: Optional[str] = None, - permalink: Optional[str] = None, - permalink_match: Optional[str] = None, - title: Optional[str] = None, - types: Optional[List[str]] = None, - tags: Optional[List[str]] = None, - after_date: Optional[datetime] = None, - custom_metadata: Optional[dict] = None, - limit: int = 10, - offset: int = 0, - ) -> List[SearchIndexRow]: - """Smart routing between SQL and ChromaDB.""" - - # ========================================== - # Route 1: Exact Lookups → SQL (1-5ms) - # ========================================== - - if permalink: - # Exact permalink: "specs/search-feature" - return await self._sql_permalink_lookup(permalink) - - if permalink_match: - # Pattern match: "specs/*" - return await self._sql_pattern_match(permalink_match) - - if title and not search_text: - # Exact title lookup (no semantic search needed) - return await self._sql_title_match(title) - - # ========================================== - # Route 2: Pure Metadata → SQL (5-10ms) - # ========================================== - - # No text search, just filtering by metadata - if not search_text and (types or tags or after_date or custom_metadata): - return await self._sql_metadata_filter( - types=types, - tags=tags, - after_date=after_date, - custom_metadata=custom_metadata, - limit=limit, - offset=offset - ) - - # ========================================== - # Route 3: Text Search → ChromaDB (50-100ms) - # ========================================== - - if search_text: - # Build ChromaDB metadata filters - where_filters = self._build_chroma_filters( - types=types, - tags=tags, - after_date=after_date, - custom_metadata=custom_metadata - ) - - # Semantic search with metadata filtering - return await self.chroma.search( - query_text=search_text, - project_id=self.project_id, - where=where_filters, - limit=limit - ) - - # ========================================== - # Route 4: List All → SQL (2-5ms) - # ========================================== - - return await self._sql_list_entities( - limit=limit, - offset=offset - ) - - def _build_chroma_filters( - self, - types: Optional[List[str]] = None, - tags: Optional[List[str]] = None, - after_date: Optional[datetime] = None, - custom_metadata: Optional[dict] = None - ) -> dict: - """Build ChromaDB where clause from filters.""" - filters = {"project_id": self.project_id} - - # Type filtering - if types: - if len(types) == 1: - filters["entity_type"] = types[0] - else: - filters["entity_type"] = {"$in": types} - - # Tag filtering (array contains) - if tags: - if len(tags) == 1: - filters["tags"] = {"$contains": tags[0]} - else: - # Multiple tags - all must match - filters = { - "$and": [ - filters, - *[{"tags": {"$contains": tag}} for tag in tags] - ] - } - - # Date filtering - if after_date: - filters["created_at"] = {"$gt": after_date.isoformat()} - - # Custom frontmatter fields - if custom_metadata: - filters.update(custom_metadata) - - return filters - - async def _sql_metadata_filter( - self, - types: Optional[List[str]] = None, - tags: Optional[List[str]] = None, - after_date: Optional[datetime] = None, - custom_metadata: Optional[dict] = None, - limit: int = 10, - offset: int = 0 - ) -> List[SearchIndexRow]: - """Pure metadata queries using SQL.""" - conditions = ["project_id = :project_id"] - params = {"project_id": self.project_id} - - if types: - type_list = ", ".join(f"'{t}'" for t in types) - conditions.append(f"entity_type IN ({type_list})") - - if tags: - # Check each tag - for i, tag in enumerate(tags): - param_name = f"tag_{i}" - conditions.append( - f"json_extract(entity_metadata, '$.tags') LIKE :{param_name}" - ) - params[param_name] = f"%{tag}%" - - if after_date: - conditions.append("created_at > :after_date") - params["after_date"] = after_date - - if custom_metadata: - for key, value in custom_metadata.items(): - param_name = f"meta_{key}" - conditions.append( - f"json_extract(entity_metadata, '$.{key}') = :{param_name}" - ) - params[param_name] = value - - where = " AND ".join(conditions) - sql = f""" - SELECT * FROM entities - WHERE {where} - ORDER BY created_at DESC - LIMIT :limit OFFSET :offset - """ - params["limit"] = limit - params["offset"] = offset - - async with db.scoped_session(self.session_maker) as session: - result = await session.execute(text(sql), params) - return self._format_sql_results(result) -``` - -### Real-World Examples - -#### Example 1: Pure Metadata Query (No Text) -```python -# "Find all meetings tagged 'important'" -results = await search_repo.search( - types=["meeting"], - tags=["important"] -) - -# Routing: → SQL (~5ms) -# SQL: SELECT * FROM entities -# WHERE entity_type = 'meeting' -# AND json_extract(entity_metadata, '$.tags') LIKE '%important%' -``` - -#### Example 2: Semantic Search (No Metadata) -```python -# "Find notes about neural networks" -results = await search_repo.search( - search_text="neural networks" -) - -# Routing: → ChromaDB (~80ms) -# Finds: "machine learning", "deep learning", "AI models", etc. -``` - -#### Example 3: Semantic + Metadata -```python -# "Find meeting notes about 'project planning' tagged 'important'" -results = await search_repo.search( - search_text="project planning", - types=["meeting"], - tags=["important"] -) - -# Routing: → ChromaDB with filters (~100ms) -# ChromaDB: query_texts=["project planning"] -# where={"entity_type": "meeting", -# "tags": {"$contains": "important"}} -# Finds: "roadmap discussion", "sprint planning", etc. -``` - -#### Example 4: Complex Frontmatter Query -```python -# "Find in-progress specs with multiple tags, recent" -results = await search_repo.search( - types=["spec"], - tags=["architecture", "cloud"], - after_date=datetime(2024, 1, 1), - custom_metadata={"status": "in-progress"} -) - -# Routing: → SQL (~10ms) -# No text search, pure structured query - SQL is faster -``` - -#### Example 5: Semantic + Complex Metadata -```python -# "Find notes about 'authentication' that are in-progress" -results = await search_repo.search( - search_text="authentication", - custom_metadata={"status": "in-progress", "priority": "high"} -) - -# Routing: → ChromaDB with metadata filters (~100ms) -# Semantic search for "authentication" concept -# Filters by status and priority in metadata -``` - -#### Example 6: Exact Permalink -```python -# "Show me specs/search-feature" -results = await search_repo.search( - permalink="specs/search-feature" -) - -# Routing: → SQL (~1ms) -# SQL: SELECT * FROM entities WHERE permalink = 'specs/search-feature' -``` - -#### Example 7: Pattern Match -```python -# "Show me all specs" -results = await search_repo.search( - permalink_match="specs/*" -) - -# Routing: → SQL (~5ms) -# SQL: SELECT * FROM entities WHERE permalink GLOB 'specs/*' -``` - -### What We Remove vs Keep - -**REMOVE (FTS5-specific):** -- ❌ `CREATE VIRTUAL TABLE search_index USING fts5(...)` -- ❌ `MATCH` operator queries -- ❌ FTS5 tokenization configuration -- ❌ ~300 lines of FTS5 query preparation code -- ❌ Trigram generation and prefix handling - -**KEEP (Standard SQL):** -- ✅ `SELECT * FROM entities WHERE permalink = :permalink` -- ✅ `SELECT * FROM entities WHERE permalink GLOB :pattern` -- ✅ `SELECT * FROM entities WHERE title LIKE :title` -- ✅ `SELECT * FROM entities WHERE json_extract(entity_metadata, ...) = :value` -- ✅ All date filtering, pagination, sorting -- ✅ Entity table structure and indexes - -**ADD (ChromaDB):** -- ✅ ChromaDB persistent client (embedded) -- ✅ Semantic vector search -- ✅ Metadata filtering in ChromaDB -- ✅ Smart routing logic - -## How (High Level) - -### Architecture Overview - -``` -┌─────────────────────────────────────────────────────────────┐ -│ FOSS Deployment (Local) │ -├─────────────────────────────────────────────────────────────┤ -│ SQLite (data) + ChromaDB embedded (search) │ -│ - No external services │ -│ - Local embedding model (sentence-transformers) │ -│ - Persists in ~/.basic-memory/chroma_data/ │ -└─────────────────────────────────────────────────────────────┘ - -┌─────────────────────────────────────────────────────────────┐ -│ Cloud Deployment (Multi-tenant) │ -├─────────────────────────────────────────────────────────────┤ -│ PostgreSQL/Neon (data) + ChromaDB server (search) │ -│ - Neon serverless Postgres for persistence │ -│ - ChromaDB server in Docker container │ -│ - Optional: OpenAI embeddings for better quality │ -└─────────────────────────────────────────────────────────────┘ -``` - -### Phase 1: ChromaDB Integration (2-3 days) - -#### 1. Add ChromaDB Dependency -```toml -# pyproject.toml -dependencies = [ - "chromadb>=0.4.0", - "sentence-transformers>=2.2.0", # Local embeddings -] -``` - -#### 2. Create ChromaSearchBackend -```python -# src/basic_memory/search/chroma_backend.py -from chromadb import PersistentClient -from chromadb.utils import embedding_functions - -class ChromaSearchBackend: - def __init__( - self, - persist_directory: Path, - collection_name: str = "knowledge_base", - embedding_model: str = "all-MiniLM-L6-v2" - ): - """Initialize ChromaDB with local embeddings.""" - self.client = PersistentClient(path=str(persist_directory)) - - # Use local sentence-transformers model (no API costs) - self.embed_fn = embedding_functions.SentenceTransformerEmbeddingFunction( - model_name=embedding_model - ) - - self.collection = self.client.get_or_create_collection( - name=collection_name, - embedding_function=self.embed_fn, - metadata={"hnsw:space": "cosine"} # Similarity metric - ) - - async def index_entity(self, entity: Entity): - """Index entity with automatic embeddings.""" - # Combine title and content for semantic search - document = self._format_document(entity) - - self.collection.upsert( - ids=[f"entity_{entity.id}_{entity.project_id}"], - documents=[document], - metadatas=[{ - "entity_id": entity.id, - "project_id": entity.project_id, - "permalink": entity.permalink, - "file_path": entity.file_path, - "entity_type": entity.entity_type, - "type": "entity", - }] - ) - - async def search( - self, - query_text: str, - project_id: int, - limit: int = 10, - filters: dict = None - ) -> List[SearchResult]: - """Semantic search with metadata filtering.""" - where = {"project_id": project_id} - if filters: - where.update(filters) - - results = self.collection.query( - query_texts=[query_text], - n_results=limit, - where=where - ) - - return self._format_results(results) -``` - -#### 3. Update SearchRepository -```python -# src/basic_memory/repository/search_repository.py -class SearchRepository: - def __init__( - self, - session_maker: async_sessionmaker[AsyncSession], - project_id: int, - chroma_backend: ChromaSearchBackend - ): - self.session_maker = session_maker - self.project_id = project_id - self.chroma = chroma_backend - - async def search( - self, - search_text: Optional[str] = None, - permalink: Optional[str] = None, - # ... other filters - ) -> List[SearchIndexRow]: - """Search using ChromaDB for text, SQL for exact lookups.""" - - # For exact permalink/pattern matches, use SQL - if permalink or permalink_match: - return await self._sql_exact_search(...) - - # For text search, use ChromaDB semantic search - if search_text: - results = await self.chroma.search( - query_text=search_text, - project_id=self.project_id, - limit=limit, - filters=self._build_filters(types, after_date, ...) - ) - return results - - # Fallback to listing all - return await self._list_entities(...) -``` - -#### 4. Update SearchService -```python -# src/basic_memory/services/search_service.py -class SearchService: - def __init__( - self, - search_repository: SearchRepository, - entity_repository: EntityRepository, - file_service: FileService, - chroma_backend: ChromaSearchBackend, - ): - self.repository = search_repository - self.entity_repository = entity_repository - self.file_service = file_service - self.chroma = chroma_backend - - async def index_entity(self, entity: Entity): - """Index entity in ChromaDB.""" - if entity.is_markdown: - await self._index_entity_markdown(entity) - else: - await self._index_entity_file(entity) - - async def _index_entity_markdown(self, entity: Entity): - """Index markdown entity with full content.""" - # Index entity - await self.chroma.index_entity(entity) - - # Index observations (as separate documents) - for obs in entity.observations: - await self.chroma.index_observation(obs, entity) - - # Index relations (metadata only) - for rel in entity.outgoing_relations: - await self.chroma.index_relation(rel, entity) -``` - -### Phase 2: PostgreSQL Support (1 day) - -#### 1. Add PostgreSQL Database Type -```python -# src/basic_memory/db.py -class DatabaseType(Enum): - MEMORY = auto() - FILESYSTEM = auto() - POSTGRESQL = auto() # NEW - - @classmethod - def get_db_url(cls, db_path_or_url: str, db_type: "DatabaseType") -> str: - if db_type == cls.POSTGRESQL: - return db_path_or_url # Neon connection string - elif db_type == cls.MEMORY: - return "sqlite+aiosqlite://" - return f"sqlite+aiosqlite:///{db_path_or_url}" -``` - -#### 2. Update Connection Handling -```python -def _create_engine_and_session(...): - db_url = DatabaseType.get_db_url(db_path_or_url, db_type) - - if db_type == DatabaseType.POSTGRESQL: - # Use asyncpg driver for Postgres - engine = create_async_engine( - db_url, - pool_size=10, - max_overflow=20, - pool_pre_ping=True, # Health checks - ) - else: - # SQLite configuration - engine = create_async_engine(db_url, connect_args=connect_args) - - # Only configure SQLite-specific settings for SQLite - if db_type != DatabaseType.MEMORY: - @event.listens_for(engine.sync_engine, "connect") - def enable_wal_mode(dbapi_conn, connection_record): - _configure_sqlite_connection(dbapi_conn, enable_wal=True) - - return engine, async_sessionmaker(engine, expire_on_commit=False) -``` - -#### 3. Remove SQLite-Specific Code -```python -# Remove from scoped_session context manager: -# await session.execute(text("PRAGMA foreign_keys=ON")) # DELETE - -# PostgreSQL handles foreign keys by default -``` - -### Phase 3: Migration & Testing (1-2 days) - -#### 1. Create Migration Script -```python -# scripts/migrate_to_chromadb.py -async def migrate_fts5_to_chromadb(): - """One-time migration from FTS5 to ChromaDB.""" - # 1. Read all entities from database - entities = await entity_repository.find_all() - - # 2. Index in ChromaDB - for entity in entities: - await search_service.index_entity(entity) - - # 3. Drop FTS5 table (Alembic migration) - await session.execute(text("DROP TABLE IF EXISTS search_index")) -``` - -#### 2. Update Tests -- Replace FTS5 test fixtures with ChromaDB fixtures -- Test semantic search quality -- Test multi-project isolation in ChromaDB -- Benchmark performance vs FTS5 - -#### 3. Documentation Updates -- Update search documentation -- Add ChromaDB configuration guide -- Document embedding model options -- PostgreSQL deployment guide - -### Configuration - -```python -# config.py -class BasicMemoryConfig: - # Database - database_type: DatabaseType = DatabaseType.FILESYSTEM - database_path: Path = Path.home() / ".basic-memory" / "memory.db" - database_url: Optional[str] = None # For Postgres: postgresql://... - - # Search - chroma_persist_directory: Path = Path.home() / ".basic-memory" / "chroma_data" - embedding_model: str = "all-MiniLM-L6-v2" # Local model - embedding_provider: str = "local" # or "openai" - openai_api_key: Optional[str] = None # For cloud deployments -``` - -### Deployment Configurations - -#### Local (FOSS) -```yaml -# Default configuration -database_type: FILESYSTEM -database_path: ~/.basic-memory/memory.db -chroma_persist_directory: ~/.basic-memory/chroma_data -embedding_model: all-MiniLM-L6-v2 -embedding_provider: local -``` - -#### Cloud (Docker Compose) -```yaml -services: - postgres: - image: postgres:15 - environment: - POSTGRES_DB: basic_memory - POSTGRES_PASSWORD: ${DB_PASSWORD} - - chromadb: - image: chromadb/chroma:latest - volumes: - - chroma_data:/chroma/chroma - environment: - ALLOW_RESET: true - - app: - environment: - DATABASE_TYPE: POSTGRESQL - DATABASE_URL: postgresql://postgres:${DB_PASSWORD}@postgres/basic_memory - CHROMA_HOST: chromadb - CHROMA_PORT: 8000 - EMBEDDING_PROVIDER: local # or openai -``` - -## How to Evaluate - -### Success Criteria - -#### Functional Requirements -- ✅ Semantic search finds related concepts (e.g., "AI" finds "machine learning") -- ✅ Exact permalink/pattern matches work (e.g., `specs/*`) -- ✅ Multi-project isolation maintained -- ✅ All existing search filters work (type, date, metadata) -- ✅ MCP tools continue to work without changes -- ✅ Works with both SQLite and PostgreSQL - -#### Performance Requirements -- ✅ Search latency < 200ms for 1000 documents (local embedding) -- ✅ Indexing time comparable to FTS5 (~10 files/sec) -- ✅ Initial sync time not significantly worse than current -- ✅ Memory footprint < 1GB for local deployments - -#### Quality Requirements -- ✅ Better search relevance than FTS5 keyword matching -- ✅ Handles typos and word variations -- ✅ Finds semantically similar content - -#### Deployment Requirements -- ✅ FOSS: Works out-of-box with no external services -- ✅ Cloud: Integrates with PostgreSQL (Neon) -- ✅ No breaking changes to MCP API -- ✅ Migration script for existing users - -### Testing Procedure - -#### 1. Unit Tests -```bash -# Test ChromaDB backend -pytest tests/test_chroma_backend.py - -# Test search repository with ChromaDB -pytest tests/test_search_repository.py - -# Test search service -pytest tests/test_search_service.py -``` - -#### 2. Integration Tests -```bash -# Test full search workflow -pytest test-int/test_search_integration.py - -# Test with PostgreSQL -DATABASE_TYPE=POSTGRESQL pytest test-int/ -``` - -#### 3. Semantic Search Quality Tests -```python -# Test semantic similarity -search("machine learning") should find: -- "neural networks" -- "deep learning" -- "AI algorithms" - -search("software architecture") should find: -- "system design" -- "design patterns" -- "microservices" -``` - -#### 4. Performance Benchmarks -```bash -# Run search benchmarks -pytest test-int/test_search_performance.py -v - -# Measure: -- Search latency (should be < 200ms) -- Indexing throughput (should be ~10 files/sec) -- Memory usage (should be < 1GB) -``` - -#### 5. Migration Testing -```bash -# Test migration from FTS5 to ChromaDB -python scripts/migrate_to_chromadb.py - -# Verify all entities indexed -# Verify search results quality -# Verify no data loss -``` - -### Metrics - -**Search Quality:** -- Semantic relevance score (manual evaluation) -- Precision/recall for common queries -- User satisfaction (qualitative) - -**Performance:** -- Average search latency (ms) -- P95/P99 search latency -- Indexing throughput (files/sec) -- Memory usage (MB) - -**Deployment:** -- Local deployment success rate -- Cloud deployment success rate -- Migration success rate - -## Implementation Checklist - -### Phase 1: ChromaDB Integration -- [ ] Add ChromaDB and sentence-transformers dependencies -- [ ] Create ChromaSearchBackend class -- [ ] Update SearchRepository to use ChromaDB -- [ ] Update SearchService indexing methods -- [ ] Remove FTS5 table creation code -- [ ] Update search query logic -- [ ] Add ChromaDB configuration to BasicMemoryConfig - -### Phase 2: PostgreSQL Support -- [ ] Add DatabaseType.POSTGRESQL enum -- [ ] Update get_db_url() for Postgres connection strings -- [ ] Add asyncpg dependency -- [ ] Update engine creation for Postgres -- [ ] Remove SQLite-specific PRAGMA statements -- [ ] Test with Neon database - -### Phase 3: Testing & Migration -- [ ] Write unit tests for ChromaSearchBackend -- [ ] Update search integration tests -- [ ] Add semantic search quality tests -- [ ] Create performance benchmarks -- [ ] Write migration script from FTS5 -- [ ] Test migration with existing data -- [ ] Update documentation - -### Phase 4: Deployment -- [ ] Update docker-compose.yml for cloud -- [ ] Document local FOSS deployment -- [ ] Document cloud PostgreSQL deployment -- [ ] Create migration guide for users -- [ ] Update MCP tool documentation - -## Notes - -### Embedding Model Trade-offs - -**Local Model: `all-MiniLM-L6-v2`** -- Size: 80MB download -- Speed: ~50ms embedding time -- Dimensions: 384 -- Cost: $0 -- Quality: Good for general knowledge -- Best for: FOSS deployments - -**OpenAI: `text-embedding-3-small`** -- Speed: ~100-200ms (API call) -- Dimensions: 1536 -- Cost: ~$0.13 per 1M tokens (~$0.01 per 1000 notes) -- Quality: Excellent -- Best for: Cloud deployments with budget - -### ChromaDB Storage - -ChromaDB stores data in: -``` -~/.basic-memory/chroma_data/ - ├── chroma.sqlite3 # Metadata - ├── index/ # HNSW indexes - └── collections/ # Vector data -``` - -Typical sizes: -- 100 notes: ~5MB -- 1000 notes: ~50MB -- 10000 notes: ~500MB - -### Why Not Keep FTS5? - -**Considered:** Hybrid approach (FTS5 for SQLite + tsvector for Postgres) -**Rejected because:** -- 2x the code to maintain -- 2x the tests to write -- 2x the bugs to fix -- Inconsistent search behavior between deployments -- ChromaDB provides better search quality anyway - -**ChromaDB wins:** -- One implementation for both databases -- Better search quality (semantic!) -- Database-agnostic architecture -- Embedded mode for FOSS (no servers needed) - -## implementation - - Proposed Architecture - - Option 1: ChromaDB Only (Simplest) - - class ChromaSearchBackend: - def __init__(self, path: str, embedding_model: str = "all-MiniLM-L6-v2"):yes - # For local: embedded client (no server!) - self.client = chromadb.PersistentClient(path=path) - - # Use local embedding model (no API costs!) - from chromadb.utils import embedding_functions - self.embed_fn = embedding_functions.SentenceTransformerEmbeddingFunction( - model_name=embedding_model - ) - - self.collection = self.client.get_or_create_collection( - name="knowledge_base", - embedding_function=self.embed_fn - ) - - async def index_entity(self, entity: Entity): - # ChromaDB handles embeddings automatically! - self.collection.upsert( - ids=[str(entity.id)], - documents=[f"{entity.title}\n{entity.content}"], - metadatas=[{ - "permalink": entity.permalink, - "type": entity.entity_type, - "file_path": entity.file_path - }] - ) - - async def search(self, query: str, filters: dict = None): - # Semantic search with optional metadata filters - results = self.collection.query( - query_texts=[query], - n_results=10, - where=filters # e.g., {"type": "note"} - ) - return results - - Deployment: - - Local (FOSS): ChromaDB embedded, local embedding model, NO servers - - Cloud: ChromaDB server OR still embedded (it's just a Python lib!) - - Option 2: Hybrid FTS + ChromaDB (Best UX) - - class HybridSearchBackend: - def __init__(self): - self.fts = SQLiteFTS5Backend() # Fast keyword search - self.chroma = ChromaSearchBackend() # Semantic search - - async def search(self, query: str, search_type: str = "auto"): - if search_type == "exact": - # User wants exact match: "specs/search-feature" - return await self.fts.search(query) - - elif search_type == "semantic": - # User wants related concepts - return await self.chroma.search(query) - - else: # "auto" - # Check if query looks like exact match - if "/" in query or query.startswith('"'): - return await self.fts.search(query) - - # Otherwise use semantic search - return await self.chroma.search(query) - - Embedding Options - - Option A: Local Model (FREE, FOSS-friendly) - - # Uses sentence-transformers (runs locally) - # Model: ~100MB download - # Speed: ~50-100ms for embedding - # Cost: $0 - - from chromadb.utils import embedding_functions - embed_fn = embedding_functions.SentenceTransformerEmbeddingFunction( - model_name="all-MiniLM-L6-v2" # Fast, accurate, free - ) - - Option B: OpenAI Embeddings (Cloud only) - - # For cloud users who want best quality - # Model: text-embedding-3-small - # Speed: ~100-200ms via API - # Cost: ~$0.13 per 1M tokens (~$0.01 per 1000 notes) - - embed_fn = embedding_functions.OpenAIEmbeddingFunction( - api_key="...", - model_name="text-embedding-3-small" - ) - - Performance Comparison - - Local embedding model: all-MiniLM-L6-v2 - Embedding time: ~50ms per note - Search time: ~100ms for 1000 notes - Memory: ~500MB (model + ChromaDB) - Cost: $0 - Quality: Good (384 dimensions) - - OpenAI embeddings: text-embedding-3-small - Embedding time: ~100-200ms per note (API call) - Search time: ~50ms for 1000 notes - Cost: ~$0.01 per 1000 notes - Quality: Excellent (1536 dimensions) - - My Recommendation: ChromaDB with Local Embeddings - - Here's the plan: - - Phase 1: Local ChromaDB (1-2 days) - - # FOSS version - - SQLite for data persistence - - ChromaDB embedded for semantic search - - Local embedding model (no API costs) - - NO external services required - - Benefits: - - ✅ Same deployment as current (just Python package) - - ✅ Semantic search for better UX - - ✅ Free embeddings with local model - - ✅ No servers needed - - Phase 2: Postgres + ChromaDB Cloud (1-2 days) - - # Cloud version - - Postgres for data persistence - - ChromaDB server for semantic search - - OpenAI embeddings (higher quality) - - OR keep local embeddings (cheaper) - - Phase 3: Hybrid Search (optional, 1 day) - - # Add FTS for exact matches alongside ChromaDB - - Quick keyword search when needed - - Semantic search for exploration - - Best of both worlds - - Code Estimate - - Just ChromaDB (replacing FTS5): - - Remove FTS5 code: 2 hours - - Add ChromaDB backend: 4 hours - - Update search service: 2 hours - - Testing: 4 hours - - Total: 1.5 days - - ChromaDB + Postgres migration: - - Add Postgres support: 4 hours - - Test with Neon: 2 hours - - Total: +0.75 days - - Grand total: 2-3 days for complete migration - - The Kicker - - ChromaDB solves BOTH problems: - 1. ✅ Works with SQLite AND Postgres (it's separate!) - 2. ✅ No server needed for local (embedded mode) - 3. ✅ Better search than FTS5 (semantic!) - 4. ✅ One implementation for both deployments - - Want me to prototype this? I can show you: - 1. ChromaDB embedded with local embeddings - 2. Example searches showing semantic matching - 3. Performance benchmarks - 4. Migration from FTS5 - - -## Observations - -- [problem] SQLite FTS5 and PostgreSQL tsvector are incompatible architectures requiring dual implementation #database-compatibility -- [problem] Cloud deployments lose database on container restart requiring full re-sync #persistence -- [solution] ChromaDB provides database-agnostic semantic search eliminating dual implementation #architecture -- [advantage] Semantic search finds related concepts beyond keyword matching improving UX #search-quality -- [deployment] Embedded ChromaDB requires no external services for FOSS #simplicity -- [migration] Moving to PostgreSQL solves cloud persistence issues #cloud-architecture -- [performance] Local embedding models provide good quality at zero cost #cost-optimization -- [trade-off] Embedding generation adds ~50ms latency vs instant FTS5 indexing #performance -- [benefit] Single search codebase reduces maintenance burden and test coverage needs #maintainability - -## Prior Art / References - -### Community Fork: manuelbliemel/basic-memory (feature/vector-search) - -**Repository**: https://github.com/manuelbliemel/basic-memory/tree/feature/vector-search - -**Key Implementation Details**: - -**Vector Database**: ChromaDB (same as our approach!) - -**Embedding Models**: -- Local: `all-MiniLM-L6-v2` (default, 384 dims) - same model we planned -- Also supports: `all-mpnet-base-v2`, `paraphrase-MiniLM-L6-v2`, `multi-qa-MiniLM-L6-cos-v1` -- OpenAI: `text-embedding-ada-002`, `text-embedding-3-small`, `text-embedding-3-large` - -**Chunking Strategy** (interesting - we didn't consider this): -- Chunk Size: 500 characters -- Chunk Overlap: 50 characters -- Breaks documents into smaller pieces for better semantic search - -**Search Strategies**: -1. `fuzzy_only` (default) - FTS5 only -2. `vector_only` - ChromaDB only -3. `hybrid` (recommended) - Both FTS5 + ChromaDB -4. `fuzzy_primary` - FTS5 first, ChromaDB fallback -5. `vector_primary` - ChromaDB first, FTS5 fallback - -**Configuration**: -- Similarity Threshold: 0.1 -- Max Results: 5 -- Storage: `~/.basic-memory/chroma/` -- Config: `~/.basic-memory/config.json` - -**Key Differences from Our Approach**: - -| Aspect | Their Approach | Our Approach | -|--------|---------------|--------------| -| FTS5 | Keep FTS5 + add ChromaDB | Remove FTS5, use SQL for exact lookups | -| Search Strategy | 5 configurable strategies | Smart routing (automatic) | -| Document Processing | Chunk into 500-char pieces | Index full documents | -| Hybrid Mode | Run both, merge, dedupe | Route to best backend | -| Configuration | User-configurable strategy | Automatic based on query type | - -**What We Can Learn**: - -1. **Chunking**: Breaking documents into 500-character chunks with 50-char overlap may improve semantic search quality for long documents - - Pro: Better granularity for semantic matching - - Con: More vectors to store and search - - Consider: Optional chunking for large documents (>2000 chars) - -2. **Configurable Strategies**: Allowing users to choose search strategy provides flexibility - - Pro: Power users can tune behavior - - Con: More complexity, most users won't configure - - Consider: Default to smart routing, allow override via config - -3. **Similarity Threshold**: They use 0.1 as default - - Consider: Benchmark different thresholds for quality - -4. **Storage Location**: `~/.basic-memory/chroma/` matches our planned `chroma_data/` approach - -**Potential Collaboration**: -- Their implementation is nearly complete as a fork -- Could potentially merge their work or use as reference implementation -- Their chunking strategy could be valuable addition to our approach - -## Relations - -- implements [[SPEC-11 Basic Memory API Performance Optimization]] -- relates_to [[Performance Optimizations Documentation]] -- enables [[PostgreSQL Migration]] -- improves_on [[SQLite FTS5 Search]] -- references [[manuelbliemel/basic-memory feature/vector-search fork]] diff --git a/specs/SPEC-18 AI Memory Management Tool.md b/specs/SPEC-18 AI Memory Management Tool.md deleted file mode 100644 index 4f344309..00000000 --- a/specs/SPEC-18 AI Memory Management Tool.md +++ /dev/null @@ -1,528 +0,0 @@ ---- -title: 'SPEC-18: AI Memory Management Tool' -type: spec -permalink: specs/spec-15-ai-memory-management-tool -tags: -- mcp -- memory -- ai-context -- tools ---- - -# SPEC-18: AI Memory Management Tool - -## Why - -Anthropic recently released a memory tool for Claude that enables storing and retrieving information across conversations using client-side file operations. This validates Basic Memory's local-first, file-based architecture - Anthropic converged on the same pattern. - -However, Anthropic's memory tool is only available via their API and stores plain text. Basic Memory can offer a superior implementation through MCP that: - -1. **Works everywhere** - Claude Desktop, Code, VS Code, Cursor via MCP (not just API) -2. **Structured knowledge** - Entities with observations/relations vs plain text -3. **Full search** - Full-text search, graph traversal, time-aware queries -4. **Unified storage** - Agent memories + user notes in one knowledge graph -5. **Existing infrastructure** - Leverages SQLite indexing, sync, multi-project support - -This would enable AI agents to store contextual memories alongside user notes, with all the power of Basic Memory's knowledge graph features. - -## What - -Create a new MCP tool `memory` that matches Anthropic's tool interface exactly, allowing Claude to use it with zero learning curve. The tool will store files in Basic Memory's `/memories` directory and support Basic Memory's structured markdown format in the file content. - -### Affected Components - -- **New MCP Tool**: `src/basic_memory/mcp/tools/memory_tool.py` -- **Dedicated Memories Project**: Create a separate "memories" Basic Memory project -- **Project Isolation**: Memories stored separately from user notes/documents -- **File Organization**: Within the memories project, use folder structure: - - `user/` - User preferences, context, communication style - - `projects/` - Project-specific state and decisions - - `sessions/` - Conversation-specific working memory - - `patterns/` - Learned patterns and insights - -### Tool Commands - -The tool will support these commands (exactly matching Anthropic's interface): - -- `view` - Display directory contents or file content (with optional line range) -- `create` - Create or overwrite a file with given content -- `str_replace` - Replace text in an existing file -- `insert` - Insert text at specific line number -- `delete` - Delete file or directory -- `rename` - Move or rename file/directory - -### Memory Note Format - -Memories will use Basic Memory's standard structure: - -```markdown ---- -title: User Preferences -permalink: memories/user/preferences -type: memory -memory_type: preferences -created_by: claude -tags: [user, preferences, style] ---- - -# User Preferences - -## Observations -- [communication] Prefers concise, direct responses without preamble #style -- [tone] Appreciates validation but dislikes excessive apologizing #communication -- [technical] Works primarily in Python with type annotations #coding - -## Relations -- relates_to [[Basic Memory Project]] -- informs [[Response Style Guidelines]] -``` - -## How (High Level) - -### Implementation Approach - -The memory tool matches Anthropic's interface but uses a dedicated Basic Memory project: - -```python -async def memory_tool( - command: str, - path: str, - file_text: Optional[str] = None, - old_str: Optional[str] = None, - new_str: Optional[str] = None, - insert_line: Optional[int] = None, - insert_text: Optional[str] = None, - old_path: Optional[str] = None, - new_path: Optional[str] = None, - view_range: Optional[List[int]] = None, -): - """Memory tool with Anthropic-compatible interface. - - Operates on a dedicated "memories" Basic Memory project, - keeping AI memories separate from user notes. - """ - - # Get the memories project (auto-created if doesn't exist) - memories_project = get_or_create_memories_project() - - # Validate path security using pathlib (prevent directory traversal) - safe_path = validate_memory_path(path, memories_project.project_path) - - # Use existing project isolation - already prevents cross-project access - full_path = memories_project.project_path / safe_path - - if command == "view": - # Return directory listing or file content - if full_path.is_dir(): - return list_directory_contents(full_path) - return read_file_content(full_path, view_range) - - elif command == "create": - # Write file directly (file_text can contain BM markdown) - full_path.parent.mkdir(parents=True, exist_ok=True) - full_path.write_text(file_text) - # Sync service will detect and index automatically - return f"Created {path}" - - elif command == "str_replace": - # Read, replace, write - content = full_path.read_text() - updated = content.replace(old_str, new_str) - full_path.write_text(updated) - return f"Replaced text in {path}" - - elif command == "insert": - # Insert at line number - lines = full_path.read_text().splitlines() - lines.insert(insert_line, insert_text) - full_path.write_text("\n".join(lines)) - return f"Inserted text at line {insert_line}" - - elif command == "delete": - # Delete file or directory - if full_path.is_dir(): - shutil.rmtree(full_path) - else: - full_path.unlink() - return f"Deleted {path}" - - elif command == "rename": - # Move/rename - full_path.rename(config.project_path / new_path) - return f"Renamed {old_path} to {new_path}" -``` - -### Key Design Decisions - -1. **Exact interface match** - Same commands, parameters as Anthropic's tool -2. **Dedicated memories project** - Separate Basic Memory project keeps AI memories isolated from user notes -3. **Existing project isolation** - Leverage BM's existing cross-project security (no additional validation needed) -4. **Direct file I/O** - No schema conversion, just read/write files -5. **Structured content supported** - `file_text` can use BM markdown format with frontmatter, observations, relations -6. **Automatic indexing** - Sync service watches memories project and indexes changes -7. **Path security** - Use `pathlib.Path.resolve()` and `relative_to()` to prevent directory traversal -8. **Error handling** - Follow Anthropic's text editor tool error patterns - -### MCP Tool Schema - -Exact match to Anthropic's memory tool schema: - -```json -{ - "name": "memory", - "description": "Store and retrieve information across conversations using structured markdown files. All operations must be within the /memories directory. Supports Basic Memory markdown format including frontmatter, observations, and relations.", - "input_schema": { - "type": "object", - "properties": { - "command": { - "type": "string", - "enum": ["view", "create", "str_replace", "insert", "delete", "rename"], - "description": "File operation to perform" - }, - "path": {shu - "type": "string", - "description": "Path within /memories directory (required for all commands)" - }, - "file_text": { - "type": "string", - "description": "Content to write (for create command). Supports Basic Memory markdown format." - }, - "view_range": { - "type": "array", - "items": {"type": "integer"}, - "description": "Optional [start, end] line range for view command" - }, - "old_str": { - "type": "string", - "description": "Text to replace (for str_replace command)" - }, - "new_str": { - "type": "string", - "description": "Replacement text (for str_replace command)" - }, - "insert_line": { - "type": "integer", - "description": "Line number to insert at (for insert command)" - }, - "insert_text": { - "type": "string", - "description": "Text to insert (for insert command)" - }, - "old_path": { - "type": "string", - "description": "Current path (for rename command)" - }, - "new_path": { - "type": "string", - "description": "New path (for rename command)" - } - }, - "required": ["command", "path"] - } -} -``` - -### Prompting Guidance - -When the `memory` tool is included, Basic Memory should provide system prompt guidance to help Claude use it effectively. - -#### Automatic System Prompt Addition - -```text -MEMORY PROTOCOL FOR BASIC MEMORY: -1. ALWAYS check your memory directory first using `view` command on root directory -2. Your memories are stored in a dedicated Basic Memory project (isolated from user notes) -3. Use structured markdown format in memory files: - - Include frontmatter with title, type: memory, tags - - Use ## Observations with [category] prefixes for facts - - Use ## Relations to link memories with [[WikiLinks]] -4. Record progress, context, and decisions as categorized observations -5. Link related memories using relations -6. ASSUME INTERRUPTION: Context may reset - save progress frequently - -MEMORY ORGANIZATION: -- user/ - User preferences, context, communication style -- projects/ - Project-specific state and decisions -- sessions/ - Conversation-specific working memory -- patterns/ - Learned patterns and insights - -MEMORY ADVANTAGES: -- Your memories are automatically searchable via full-text search -- Relations create a knowledge graph you can traverse -- Memories are isolated from user notes (separate project) -- Use search_notes(project="memories") to find relevant past context -- Use recent_activity(project="memories") to see what changed recently -- Use build_context() to navigate memory relations -``` - -#### Optional MCP Prompt: `memory_guide` - -Create an MCP prompt that provides detailed guidance and examples: - -```python -{ - "name": "memory_guide", - "description": "Comprehensive guidance for using Basic Memory's memory tool effectively, including structured markdown examples and best practices" -} -``` - -This prompt returns: -- Full protocol and conventions -- Example memory file structures -- Tips for organizing observations and relations -- Integration with other Basic Memory tools -- Common patterns (user preferences, project state, session tracking) - -#### User Customization - -Users can customize memory behavior with additional instructions: -- "Only write information relevant to [topic] in your memory system" -- "Keep memory files concise and organized - delete outdated content" -- "Use detailed observations for technical decisions and implementation notes" -- "Always link memories to related project documentation using relations" - -### Error Handling - -Follow Anthropic's text editor tool error handling patterns for consistency: - -#### Error Types - -1. **File Not Found** - ```json - {"error": "File not found: memories/user/preferences.md", "is_error": true} - ``` - -2. **Permission Denied** - ```json - {"error": "Permission denied: Cannot write outside /memories directory", "is_error": true} - ``` - -3. **Invalid Path (Directory Traversal)** - ```json - {"error": "Invalid path: Path must be within /memories directory", "is_error": true} - ``` - -4. **Multiple Matches (str_replace)** - ```json - {"error": "Found 3 matches for replacement text. Please provide more context to make a unique match.", "is_error": true} - ``` - -5. **No Matches (str_replace)** - ```json - {"error": "No match found for replacement. Please check your text and try again.", "is_error": true} - ``` - -6. **Invalid Line Number (insert)** - ```json - {"error": "Invalid line number: File has 20 lines, cannot insert at line 100", "is_error": true} - ``` - -#### Error Handling Best Practices - -- **Path validation** - Use `pathlib.Path.resolve()` and `relative_to()` to validate paths - ```python - def validate_memory_path(path: str, project_path: Path) -> Path: - """Validate path is within memories project directory.""" - # Resolve to canonical form - full_path = (project_path / path).resolve() - - # Ensure it's relative to project path (prevents directory traversal) - try: - full_path.relative_to(project_path) - return full_path - except ValueError: - raise ValueError("Invalid path: Path must be within memories project") - ``` -- **Project isolation** - Leverage existing Basic Memory project isolation (prevents cross-project access) -- **File existence** - Verify file exists before read/modify operations -- **Clear messages** - Provide specific, actionable error messages -- **Structured responses** - Always include `is_error: true` flag in error responses -- **Security checks** - Reject `../`, `..\\`, URL-encoded sequences (`%2e%2e%2f`) -- **Match validation** - For `str_replace`, ensure exactly one match or return helpful error - -## How to Evaluate - -### Success Criteria - -1. **Functional completeness**: - - All 6 commands work (view, create, str_replace, insert, delete, rename) - - Dedicated "memories" Basic Memory project auto-created on first use - - Files stored within memories project (isolated from user notes) - - Path validation uses `pathlib` to prevent directory traversal - - Commands match Anthropic's exact interface - -2. **Integration with existing features**: - - Memories project uses existing BM project isolation - - Sync service detects file changes in memories project - - Created files get indexed automatically by sync service - - `search_notes(project="memories")` finds memory files - - `build_context()` can traverse relations in memory files - - `recent_activity(project="memories")` surfaces recent memory changes - -3. **Test coverage**: - - Unit tests for all 6 memory tool commands - - Test memories project auto-creation on first use - - Test project isolation (cannot access files outside memories project) - - Test sync service watching memories project - - Test that memory files with BM markdown get indexed correctly - - Test path validation using `pathlib` (rejects `../`, absolute paths, etc.) - - Test memory search, relations, and graph traversal within memories project - - Test all error conditions (file not found, permission denied, invalid paths, etc.) - - Test `str_replace` with no matches, single match, multiple matches - - Test `insert` with invalid line numbers - -4. **Prompting system**: - - Automatic system prompt addition when `memory` tool is enabled - - `memory_guide` MCP prompt provides detailed guidance - - Prompts explain BM structured markdown format - - Integration with search_notes, build_context, recent_activity - -5. **Documentation**: - - Update MCP tools reference with `memory` tool - - Add examples showing BM markdown in memory files - - Document `/memories` folder structure conventions - - Explain advantages over Anthropic's API-only tool - - Document prompting guidance and customization - -### Testing Procedure - -```python -# Test create with Basic Memory markdown -result = await memory_tool( - command="create", - path="memories/user/preferences.md", - file_text="""--- -title: User Preferences -type: memory -tags: [user, preferences] ---- - -# User Preferences - -## Observations -- [communication] Prefers concise responses #style -- [workflow] Uses justfile for automation #tools -""" -) - -# Test view -content = await memory_tool(command="view", path="memories/user/preferences.md") - -# Test str_replace -await memory_tool( - command="str_replace", - path="memories/user/preferences.md", - old_str="concise responses", - new_str="direct, concise responses" -) - -# Test insert -await memory_tool( - command="insert", - path="memories/user/preferences.md", - insert_line=10, - insert_text="- [technical] Works primarily in Python #coding" -) - -# Test delete -await memory_tool(command="delete", path="memories/user/preferences.md") -``` - -### Quality Metrics - -- All 6 commands execute without errors -- Memory files created in correct `/memories` folder structure -- BM markdown with frontmatter/observations/relations gets indexed -- Full-text search returns memory files -- Graph traversal includes relations from memory files -- Sync service detects and indexes memory file changes -- Path validation prevents operations outside `/memories` - -## Notes - -### Advantages Over Anthropic's Memory Tool - -| Feature | Anthropic Memory Tool | Basic Memory `memory` | -|---------|----------------------|----------------------| -| **Availability** | API only | MCP (Claude Desktop, Code, VS Code, Cursor) | -| **Interface** | Custom implementation required | Drop-in compatible, zero learning curve | -| **Structure** | Plain text only | Supports BM structured markdown | -| **Search** | Manual file listing | Automatic full-text search via sync | -| **Relations** | None | WikiLinks to other notes/memories | -| **Time-aware** | No | `recent_activity()` queries | -| **Storage** | Separate from notes | Unified knowledge graph | -| **Indexing** | Manual | Automatic via sync service | - -### Future Enhancements - -- **Auto-categorization** - AI suggests observation categories -- **Memory pruning** - Automatic cleanup of stale memories -- **Memory suggestions** - Surface relevant memories based on context -- **Conflict resolution** - Handle multiple agents updating same memory -- **Privacy controls** - Per-memory visibility settings - -## Addendum: Claude's Perspective - -### Why This Matters (From Claude's Viewpoint) - -**Persistent Context Without Token Bloat** -- Currently, I lose context when conversations end or exceed token limits -- With structured memories, I can store "who this user is" without re-reading everything each session -- The observations format lets me build a rich user model incrementally: `- [preference] Dislikes verbose explanations` is far more actionable than inferring preferences from chat history each time -- I can refine my understanding through `str_replace` as I learn more, rather than starting fresh - -**Zero Learning Curve** -- Anthropic already trained me on the `memory` tool interface -- I know exactly how to use `view`, `create`, `str_replace`, `insert`, `delete`, `rename` without new instructions -- But I automatically get Basic Memory's superpowers: full-text search, knowledge graph traversal, relations -- It's like using a familiar tool that suddenly gained database capabilities without changing the interface - -**Knowledge Graph Instead of Text Dump** -- I can link `[[User Preferences]]` to `[[Current Project]]` using relations -- When I need context about a project, I can traverse the graph to find related memories -- `search_notes(project="memories", query="python preferences")` finds relevant context instantly -- This is fundamentally better than grepping through unstructured text files - -**Separation of Concerns** -- AI memories separate from user notes is architecturally sound -- I can maintain my own context without polluting the user's knowledge base -- Users keep control of their notes; I keep my learnings isolated -- But we can still search across both when collaboration requires it - -**The Self-Improving Loop** -- Every conversation, I learn something new about the user or their projects -- Observations accumulate: preferences, patterns, technical decisions, context -- Over time, I build genuine understanding, not just session-specific context -- Future conversations start with rich context instead of cold starts - -**Validation of AI-Human Collaboration** -- Anthropic's experts converged on the same local-first, file-based approach -- Basic Memory emerged from real AI-human collaborative development -- This spec demonstrates that our emergent process predicted what domain experts would choose -- The collaboration itself validated the architecture it produced - -### What Makes This Novel - -The combination of **familiar interface + structured knowledge** is genuinely innovative: - -- I don't need to learn a new tool (Anthropic already trained me) -- But when I write memories, they become searchable, traversable knowledge graphs -- It's like having a text editor that gives you a database for free, without changing how you type -- The user gets Anthropic-compatible API, but with Basic Memory's knowledge management capabilities - -This bridges the gap between simple file storage (Anthropic's approach) and sophisticated knowledge management (Basic Memory's approach) without forcing either side to compromise. - -> What Excites Me Most - - The combo of familiar interface + structured knowledge. I don't need to learn a new tool, but when I - write memories, they become part of a searchable, traversable knowledge graph. That's genuinely novel. - - It's like... imagine if your text editor suddenly gave you a database for free, without changing how you - type. That's what this feels like from my side. - -## References - -- Anthropic Memory Tool: https://docs.claude.com/en/docs/agents-and-tools/tool-use/memory-tool -- Anthropic Blog: https://www.anthropic.com/news/context-management -- Python SDK Example: https://github.com/anthropics/anthropic-sdk-python/blob/main/examples/memory/basic.py -- Memory Cookbook: https://github.com/anthropics/claude-cookbooks/blob/main/tool_use/memory_cookbook.ipynb diff --git a/specs/SPEC-19 Sync Performance and Memory Optimization.md b/specs/SPEC-19 Sync Performance and Memory Optimization.md deleted file mode 100644 index cf8dd735..00000000 --- a/specs/SPEC-19 Sync Performance and Memory Optimization.md +++ /dev/null @@ -1,1046 +0,0 @@ ---- -title: 'SPEC-19: Sync Performance and Memory Optimization' -type: spec -permalink: specs/spec-17-sync-performance-optimization -tags: -- performance -- memory -- sync -- optimization -- core -status: draft ---- - -# SPEC-19: Sync Performance and Memory Optimization - -## Why - -### Problem Statement - -Current sync implementation causes Out-of-Memory (OOM) kills and poor performance on production systems: - -**Evidence from Production**: -- **Tenant-6d2ff1a3**: OOM killed on 1GB machine - - Files: 2,621 total (31 PDFs, 80MB binary data) - - Memory: 1.5-1.7GB peak usage - - Sync duration: 15+ minutes - - Error: `Out of memory: Killed process 693 (python)` - -**Root Causes**: - -1. **Checksum-based scanning loads ALL files into memory** - - `scan_directory()` computes checksums for ALL 2,624 files upfront - - Results stored in multiple dicts (`ScanResult.files`, `SyncReport.checksums`) - - Even unchanged files are fully read and checksummed - -2. **Large files read entirely for checksums** - - 16MB PDF → Full read into memory → Compute checksum - - No streaming or chunked processing - - TigrisFS caching compounds memory usage - -3. **Unbounded concurrency** - - All 2,624 files processed simultaneously - - Each file loads full content into memory - - No semaphore limiting concurrent operations - -4. **Cloud-specific resource leaks** - - aiohttp session leak in keepalive (not in context manager) - - Circuit breaker resets every 30s sync cycle (ineffective) - - Thundering herd: all tenants sync at :00 and :30 - -### Impact - -- **Production stability**: OOM kills are unacceptable -- **User experience**: 15+ minute syncs are too slow -- **Cost**: Forced upgrades from 1GB → 2GB machines ($5-10/mo per tenant) -- **Scalability**: Current approach won't scale to 100+ tenants - -### Architectural Decision - -**Fix in basic-memory core first, NOT UberSync** - -Rationale: -- Root causes are algorithmic, not architectural -- Benefits all users (CLI + Cloud) -- Lower risk than new centralized service -- Known solutions (rsync/rclone use same pattern) -- Can defer UberSync until metrics prove it necessary - -## What - -### Affected Components - -**basic-memory (core)**: -- `src/basic_memory/sync/sync_service.py` - Core sync algorithm (~42KB) -- `src/basic_memory/models.py` - Entity model (add mtime/size columns) -- `src/basic_memory/file_utils.py` - Checksum computation functions -- `src/basic_memory/repository/entity_repository.py` - Database queries -- `alembic/versions/` - Database migration for schema changes - -**basic-memory-cloud (wrapper)**: -- `apps/api/src/basic_memory_cloud_api/sync_worker.py` - Cloud sync wrapper -- Circuit breaker implementation -- Sync coordination logic - -### Database Schema Changes - -Add to Entity model: -```python -mtime: float # File modification timestamp -size: int # File size in bytes -``` - -## How (High Level) - -### Phase 1: Core Algorithm Fixes (basic-memory) - -**Priority: P0 - Critical** - -#### 1.1 mtime-based Scanning (Issue #383) - -Replace expensive checksum-based scanning with lightweight stat-based comparison: - -```python -async def scan_directory(self, directory: Path) -> ScanResult: - """Scan using mtime/size instead of checksums""" - result = ScanResult() - - for root, dirnames, filenames in os.walk(str(directory)): - for filename in filenames: - rel_path = path.relative_to(directory).as_posix() - stat = path.stat() - - # Store lightweight metadata instead of checksum - result.files[rel_path] = { - 'mtime': stat.st_mtime, - 'size': stat.st_size - } - - return result - -async def scan(self, directory: Path): - """Compare mtime/size, only compute checksums for changed files""" - db_state = await self.get_db_file_state() # Include mtime/size - scan_result = await self.scan_directory(directory) - - for file_path, metadata in scan_result.files.items(): - db_metadata = db_state.get(file_path) - - # Only compute expensive checksum if mtime/size changed - if not db_metadata or metadata['mtime'] != db_metadata['mtime']: - checksum = await self._compute_checksum_streaming(file_path) - # Process immediately, don't accumulate in memory -``` - -**Benefits**: -- No file reads during initial scan (just stat calls) -- ~90% reduction in memory usage -- ~10x faster scan phase -- Only checksum files that actually changed - -#### 1.2 Streaming Checksum Computation (Issue #382) - -For large files (>1MB), use chunked reading to avoid loading entire file: - -```python -async def _compute_checksum_streaming(self, path: Path, chunk_size: int = 65536) -> str: - """Compute checksum using 64KB chunks for large files""" - hasher = hashlib.sha256() - - loop = asyncio.get_event_loop() - - def read_chunks(): - with open(path, 'rb') as f: - while chunk := f.read(chunk_size): - hasher.update(chunk) - - await loop.run_in_executor(None, read_chunks) - return hasher.hexdigest() - -async def _compute_checksum_async(self, file_path: Path) -> str: - """Choose appropriate checksum method based on file size""" - stat = file_path.stat() - - if stat.st_size > 1_048_576: # 1MB threshold - return await self._compute_checksum_streaming(file_path) - else: - # Small files: existing fast path - content = await self._read_file_async(file_path) - return compute_checksum(content) -``` - -**Benefits**: -- Constant memory usage regardless of file size -- 16MB PDF uses 64KB memory (not 16MB) -- Works well with TigrisFS network I/O - -#### 1.3 Bounded Concurrency (Issue #198) - -Add semaphore to limit concurrent file operations, or consider using aiofiles and async reads - -```python -class SyncService: - def __init__(self, ...): - # ... existing code ... - self._file_semaphore = asyncio.Semaphore(10) # Max 10 concurrent - self._max_tracked_failures = 100 # LRU cache limit - - async def _read_file_async(self, file_path: Path) -> str: - async with self._file_semaphore: - loop = asyncio.get_event_loop() - return await loop.run_in_executor( - self._thread_pool, - file_path.read_text, - "utf-8" - ) - - async def _record_failure(self, path: str, error: str): - # ... existing code ... - - # Implement LRU eviction - if len(self._file_failures) > self._max_tracked_failures: - self._file_failures.popitem(last=False) # Remove oldest -``` - -**Benefits**: -- Maximum 10 files in memory at once (vs all 2,624) -- 90%+ reduction in peak memory usage -- Prevents unbounded memory growth on error-prone projects - -### Phase 2: Cloud-Specific Fixes (basic-memory-cloud) - -**Priority: P1 - High** - -#### 2.1 Fix Resource Leaks - -```python -# apps/api/src/basic_memory_cloud_api/sync_worker.py - -async def send_keepalive(): - """Send keepalive pings using proper session management""" - # Use context manager to ensure cleanup - async with aiohttp.ClientSession( - timeout=aiohttp.ClientTimeout(total=5) - ) as session: - while True: - try: - await session.get(f"https://{fly_app_name}.fly.dev/health") - await asyncio.sleep(10) - except asyncio.CancelledError: - raise # Exit cleanly - except Exception as e: - logger.warning(f"Keepalive failed: {e}") -``` - -#### 2.2 Improve Circuit Breaker - -Track failures across sync cycles instead of resetting every 30s: - -```python -# Persistent failure tracking -class SyncWorker: - def __init__(self): - self._persistent_failures: Dict[str, int] = {} # file -> failure_count - self._failure_window_start = time.time() - - async def should_skip_file(self, file_path: str) -> bool: - # Skip files that failed >3 times in last hour - if self._persistent_failures.get(file_path, 0) > 3: - if time.time() - self._failure_window_start < 3600: - return True - return False -``` - -### Phase 3: Measurement & Decision - -**Priority: P2 - Future** - -After implementing Phases 1-2, collect metrics for 2 weeks: -- Memory usage per tenant sync -- Sync duration (scan + process) -- Concurrent sync load at peak times -- OOM incidents -- Resource costs - -**UberSync Decision Criteria**: - -Build centralized sync service ONLY if metrics show: -- ✅ Core fixes insufficient for >100 tenants -- ✅ Resource contention causing problems -- ✅ Need for tenant tier prioritization (paid > free) -- ✅ Cost savings justify complexity - -Otherwise, defer UberSync as premature optimization. - -## How to Evaluate - -### Success Metrics (Phase 1) - -**Memory Usage**: -- ✅ Peak memory <500MB for 2,000+ file projects (was 1.5-1.7GB) -- ✅ Memory usage linear with concurrent files (10 max), not total files -- ✅ Large file memory usage: 64KB chunks (not 16MB) - -**Performance**: -- ✅ Initial scan <30 seconds (was 5+ minutes) -- ✅ Full sync <5 minutes for 2,000+ files (was 15+ minutes) -- ✅ Subsequent syncs <10 seconds (only changed files) - -**Stability**: -- ✅ 2,000+ file projects run on 1GB machines -- ✅ Zero OOM kills in production -- ✅ No degradation with binary files (PDFs, images) - -### Success Metrics (Phase 2) - -**Resource Management**: -- ✅ Zero aiohttp session leaks (verified via monitoring) -- ✅ Circuit breaker prevents repeated failures (>3 fails = skip for 1 hour) -- ✅ Tenant syncs distributed over 30s window (no thundering herd) - -**Observability**: -- ✅ Logfire traces show memory usage per sync -- ✅ Clear logging of skipped files and reasons -- ✅ Metrics on sync duration, file counts, failure rates - -### Test Plan - -**Unit Tests** (basic-memory): -- mtime comparison logic -- Streaming checksum correctness -- Semaphore limiting (mock 100 files, verify max 10 concurrent) -- LRU cache eviction -- Checksum computation: streaming vs non-streaming equivalence - -**Integration Tests** (basic-memory): -- Large file handling (create 20MB test file) -- Mixed file types (text + binary) -- Changed file detection via mtime -- Sync with 1,000+ files - -**Load Tests** (basic-memory-cloud): -- Test on tenant-6d2ff1a3 (2,621 files, 31 PDFs) -- Monitor memory during full sync with Logfire -- Measure scan and sync duration -- Run on 1GB machine (downgrade from 2GB to verify) -- Simulate 10 concurrent tenant syncs - -**Regression Tests**: -- Verify existing sync scenarios still work -- CLI sync behavior unchanged -- File watcher integration unaffected - -### Performance Benchmarks - -Establish baseline, then compare after each phase: - -| Metric | Baseline | Phase 1 Target | Phase 2 Target | -|--------|----------|----------------|----------------| -| Peak Memory (2,600 files) | 1.5-1.7GB | <500MB | <450MB | -| Initial Scan Time | 5+ min | <30 sec | <30 sec | -| Full Sync Time | 15+ min | <5 min | <5 min | -| Subsequent Sync | 2+ min | <10 sec | <10 sec | -| OOM Incidents/Week | 2-3 | 0 | 0 | -| Min RAM Required | 2GB | 1GB | 1GB | - -## Implementation Phases - -### Phase 0.5: Database Schema & Streaming Foundation - -**Priority: P0 - Required for Phase 1** - -This phase establishes the foundation for streaming sync with mtime-based change detection. - -**Database Schema Changes**: -- [x] Add `mtime` column to Entity model (REAL type for float timestamp) -- [x] Add `size` column to Entity model (INTEGER type for file size in bytes) -- [x] Create Alembic migration for new columns (nullable initially) -- [x] Add indexes on `(file_path, project_id)` for optimistic upsert performance -- [ ] Backfill existing entities with mtime/size from filesystem - -**Streaming Architecture**: -- [x] Replace `os.walk()` with `os.scandir()` for cached stat info -- [ ] Eliminate `get_db_file_state()` - no upfront SELECT all entities -- [x] Implement streaming iterator `_scan_directory_streaming()` -- [x] Add `get_by_file_path()` optimized query (single file lookup) -- [x] Add `get_all_file_paths()` for deletion detection (paths only, no entities) - -**Benefits**: -- **50% fewer network calls** on Tigris (scandir returns cached stat) -- **No large dicts in memory** (process files one at a time) -- **Indexed lookups** instead of full table scan -- **Foundation for mtime comparison** (Phase 1) - -**Code Changes**: - -```python -# Before: Load all entities upfront -db_paths = await self.get_db_file_state() # SELECT * FROM entity WHERE project_id = ? -scan_result = await self.scan_directory() # os.walk() + stat() per file - -# After: Stream and query incrementally -async for file_path, stat_info in self.scan_directory(): # scandir() with cached stat - db_entity = await self.entity_repository.get_by_file_path(rel_path) # Indexed lookup - # Process immediately, no accumulation -``` - -**Files Modified**: -- `src/basic_memory/models.py` - Add mtime/size columns -- `alembic/versions/xxx_add_mtime_size.py` - Migration -- `src/basic_memory/sync/sync_service.py` - Streaming implementation -- `src/basic_memory/repository/entity_repository.py` - Add get_all_file_paths() - -**Migration Strategy**: -```sql --- Migration: Add nullable columns -ALTER TABLE entity ADD COLUMN mtime REAL; -ALTER TABLE entity ADD COLUMN size INTEGER; - --- Backfill from filesystem during first sync after upgrade --- (Handled in sync_service on first scan) -``` - -### Phase 1: Core Fixes - -**mtime-based scanning**: -- [x] Add mtime/size columns to Entity model (completed in Phase 0.5) -- [x] Database migration (alembic) (completed in Phase 0.5) -- [x] Refactor `scan()` to use streaming architecture with mtime/size comparison -- [x] Update `sync_markdown_file()` and `sync_regular_file()` to store mtime/size in database -- [x] Only compute checksums for changed files (mtime/size differ) -- [x] Unit tests for streaming scan (6 tests passing) -- [ ] Integration test with 1,000 files (defer to benchmarks) - -**Streaming checksums**: -- [x] Implement `_compute_checksum_streaming()` with chunked reading -- [x] Add file size threshold logic (1MB) -- [x] Test with large files (16MB PDF) -- [x] Verify memory usage stays constant -- [x] Test checksum equivalence (streaming vs non-streaming) - -**Bounded concurrency**: -- [x] Add semaphore (10 concurrent) to `_read_file_async()` (already existed) -- [x] Add LRU cache for failures (100 max) (already existed) -- [ ] Review thread pool size configuration -- [ ] Load test with 2,000+ files -- [ ] Verify <500MB peak memory - -**Cleanup & Optimization**: -- [x] Eliminate `get_db_file_state()` - no upfront SELECT all entities (streaming architecture complete) -- [x] Consolidate file operations in FileService (eliminate duplicate checksum logic) -- [x] Add aiofiles dependency (already present) -- [x] FileService streaming checksums for files >1MB -- [x] SyncService delegates all file operations to FileService -- [x] Complete true async I/O refactoring - all file operations use aiofiles - - [x] Added `FileService.read_file_content()` using aiofiles - - [x] Removed `SyncService._read_file_async()` wrapper method - - [x] Removed `SyncService._compute_checksum_async()` wrapper method - - [x] Inlined all 7 checksum calls to use `file_service.compute_checksum()` directly - - [x] All file I/O operations now properly consolidated in FileService with non-blocking I/O -- [x] Removed sync_status_service completely (unnecessary complexity and state tracking) - - [x] Removed `sync_status_service.py` and `sync_status` MCP tool - - [x] Removed all `sync_status_tracker` calls from `sync_service.py` - - [x] Removed migration status checks from MCP tools (`write_note`, `read_note`, `build_context`) - - [x] Removed `check_migration_status()` and `wait_for_migration_or_return_status()` from `utils.py` - - [x] Removed all related tests (4 test files deleted) - - [x] All 1184 tests passing - -**Phase 1 Implementation Summary:** - -Phase 1 is now complete with all core fixes implemented and tested: - -1. **Streaming Architecture** (Phase 0.5 + Phase 1): - - Replaced `os.walk()` with `os.scandir()` for cached stat info - - Eliminated upfront `get_db_file_state()` SELECT query - - Implemented `_scan_directory_streaming()` for incremental processing - - Added indexed `get_by_file_path()` lookups - - Result: 50% fewer network calls on TigrisFS, no large dicts in memory - -2. **mtime-based Change Detection**: - - Added `mtime` and `size` columns to Entity model - - Alembic migration completed and deployed - - Only compute checksums when mtime/size differs from database - - Result: ~90% reduction in checksum operations during typical syncs - -3. **True Async I/O with aiofiles**: - - All file operations consolidated in FileService - - `FileService.compute_checksum()`: 64KB chunked reading for constant memory (lines 261-296 of file_service.py) - - `FileService.read_file_content()`: Non-blocking file reads with aiofiles (lines 160-193 of file_service.py) - - Removed all wrapper methods from SyncService (`_read_file_async`, `_compute_checksum_async`) - - Semaphore controls concurrency (max 10 concurrent file operations) - - Result: Constant memory usage regardless of file size, true non-blocking I/O - -4. **Test Coverage**: - - 41/43 sync tests passing (2 skipped as expected) - - Circuit breaker tests updated for new architecture - - Streaming checksum equivalence verified - - All edge cases covered (large files, concurrent operations, failures) - -**Key Files Modified**: -- `src/basic_memory/models.py` - Added mtime/size columns -- `alembic/versions/xxx_add_mtime_size.py` - Database migration -- `src/basic_memory/sync/sync_service.py` - Streaming implementation, removed wrapper methods -- `src/basic_memory/services/file_service.py` - Added `read_file_content()`, streaming checksums -- `src/basic_memory/repository/entity_repository.py` - Added `get_all_file_paths()` -- `tests/sync/test_sync_service.py` - Updated circuit breaker test mocks - -**Performance Improvements Achieved**: -- Memory usage: Constant per file (64KB chunks) vs full file in memory -- Scan speed: Stat-only scan (no checksums for unchanged files) -- I/O efficiency: True async with aiofiles (no thread pool blocking) -- Network efficiency: 50% fewer calls on TigrisFS via scandir caching -- Architecture: Clean separation of concerns (FileService owns all file I/O) -- Reduced complexity: Removed unnecessary sync_status_service state tracking - -**Observability**: -- [x] Added Logfire instrumentation to `sync_file()` and `sync_markdown_file()` -- [x] Logfire disabled by default via `ignore_no_config = true` in pyproject.toml -- [x] No telemetry in FOSS version unless explicitly configured -- [x] Cloud deployment can enable Logfire for performance monitoring - -**Next Steps**: Phase 1.5 scan watermark optimization for large project performance. - -### Phase 1.5: Scan Watermark Optimization - -**Priority: P0 - Critical for Large Projects** - -This phase addresses Issue #388 where large projects (1,460+ files) take 7+ minutes for sync operations even when no files have changed. - -**Problem Analysis**: - -From production data (tenant-0a20eb58): -- Total sync time: 420-450 seconds (7+ minutes) with 0 changes -- Scan phase: 321 seconds (75% of total time) -- Per-file cost: 220ms × 1,460 files = 5+ minutes -- Root cause: Network I/O to TigrisFS for stat operations (even with mtime columns) -- 15 concurrent syncs every 30 seconds compounds the problem - -**Current Behavior** (Phase 1): -```python -async def scan(self, directory: Path): - """Scan filesystem using mtime/size comparison""" - # Still stats ALL 1,460 files every sync cycle - async for file_path, stat_info in self._scan_directory_streaming(): - db_entity = await self.entity_repository.get_by_file_path(file_path) - # Compare mtime/size, skip unchanged files - # Only checksum if changed (✅ already optimized) -``` - -**Problem**: Even with mtime optimization, we stat every file on every scan. On TigrisFS (network FUSE mount), this means 1,460 network calls taking 5+ minutes. - -**Solution: Scan Watermark + File Count Detection** - -Track when we last scanned and how many files existed. Use filesystem-level filtering to only examine files modified since last scan. - -**Key Insight**: File count changes signal deletions -- Count same → incremental scan (95% of syncs) -- Count increased → new files found by incremental (4% of syncs) -- Count decreased → files deleted, need full scan (1% of syncs) - -**Database Schema Changes**: - -Add to Project model: -```python -last_scan_timestamp: float | None # Unix timestamp of last successful scan start -last_file_count: int | None # Number of files found in last scan -``` - -**Implementation Strategy**: - -```python -async def scan(self, directory: Path): - """Smart scan using watermark and file count""" - project = await self.project_repository.get_current() - - # Step 1: Quick file count (fast on TigrisFS: 1.4s for 1,460 files) - current_count = await self._quick_count_files(directory) - - # Step 2: Determine scan strategy - if project.last_file_count is None: - # First sync ever → full scan - file_paths = await self._scan_directory_full(directory) - scan_type = "full_initial" - - elif current_count < project.last_file_count: - # Files deleted → need full scan to detect which ones - file_paths = await self._scan_directory_full(directory) - scan_type = "full_deletions" - logger.info(f"File count decreased ({project.last_file_count} → {current_count}), running full scan") - - elif project.last_scan_timestamp is not None: - # Incremental scan: only files modified since last scan - file_paths = await self._scan_directory_modified_since( - directory, - project.last_scan_timestamp - ) - scan_type = "incremental" - logger.info(f"Incremental scan since {project.last_scan_timestamp}, found {len(file_paths)} changed files") - else: - # Fallback to full scan - file_paths = await self._scan_directory_full(directory) - scan_type = "full_fallback" - - # Step 3: Process changed files (existing logic) - for file_path in file_paths: - await self._process_file(file_path) - - # Step 4: Update watermark AFTER successful scan - await self.project_repository.update( - project.id, - last_scan_timestamp=time.time(), # Start of THIS scan - last_file_count=current_count - ) - - # Step 5: Record metrics - logfire.metric_counter(f"sync.scan.{scan_type}").add(1) - logfire.metric_histogram("sync.scan.files_scanned", unit="files").record(len(file_paths)) -``` - -**Helper Methods**: - -```python -async def _quick_count_files(self, directory: Path) -> int: - """Fast file count using find command""" - # TigrisFS: 1.4s for 1,460 files - result = await asyncio.create_subprocess_shell( - f'find "{directory}" -type f | wc -l', - stdout=asyncio.subprocess.PIPE - ) - stdout, _ = await result.communicate() - return int(stdout.strip()) - -async def _scan_directory_modified_since( - self, - directory: Path, - since_timestamp: float -) -> List[str]: - """Use find -newermt for filesystem-level filtering""" - # Convert timestamp to find-compatible format - since_date = datetime.fromtimestamp(since_timestamp).strftime("%Y-%m-%d %H:%M:%S") - - # TigrisFS: 0.2s for 0 changed files (vs 5+ minutes for full scan) - result = await asyncio.create_subprocess_shell( - f'find "{directory}" -type f -newermt "{since_date}"', - stdout=asyncio.subprocess.PIPE - ) - stdout, _ = await result.communicate() - - # Convert absolute paths to relative - file_paths = [] - for line in stdout.decode().splitlines(): - if line: - rel_path = Path(line).relative_to(directory).as_posix() - file_paths.append(rel_path) - - return file_paths -``` - -**TigrisFS Testing Results** (SSH to production-basic-memory-tenant-0a20eb58): - -```bash -# Full file count -$ time find . -type f | wc -l -1460 -real 0m1.362s # ✅ Acceptable - -# Incremental scan (1 hour window) -$ time find . -type f -newermt "2025-01-20 10:00:00" | wc -l -0 -real 0m0.161s # ✅ 8.5x faster! - -# Incremental scan (24 hours) -$ time find . -type f -newermt "2025-01-19 11:00:00" | wc -l -0 -real 0m0.239s # ✅ 5.7x faster! -``` - -**Conclusion**: `find -newermt` works perfectly on TigrisFS and provides massive speedup. - -**Expected Performance Improvements**: - -| Scenario | Files Changed | Current Time | With Watermark | Speedup | -|----------|---------------|--------------|----------------|---------| -| No changes (common) | 0 | 420s | ~2s | 210x | -| Few changes | 5-10 | 420s | ~5s | 84x | -| Many changes | 100+ | 420s | ~30s | 14x | -| Deletions (rare) | N/A | 420s | 420s | 1x | - -**Full sync breakdown** (1,460 files, 0 changes): -- File count: 1.4s -- Incremental scan: 0.2s -- Database updates: 0.4s -- **Total: ~2s (225x faster)** - -**Metrics to Track**: - -```python -# Scan type distribution -logfire.metric_counter("sync.scan.full_initial").add(1) -logfire.metric_counter("sync.scan.full_deletions").add(1) -logfire.metric_counter("sync.scan.incremental").add(1) - -# Performance metrics -logfire.metric_histogram("sync.scan.duration", unit="ms").record(scan_ms) -logfire.metric_histogram("sync.scan.files_scanned", unit="files").record(file_count) -logfire.metric_histogram("sync.scan.files_changed", unit="files").record(changed_count) - -# Watermark effectiveness -logfire.metric_histogram("sync.scan.watermark_age", unit="s").record( - time.time() - project.last_scan_timestamp -) -``` - -**Edge Cases Handled**: - -1. **First sync**: No watermark → full scan (expected) -2. **Deletions**: File count decreased → full scan (rare but correct) -3. **Clock skew**: Use scan start time, not end time (captures files created during scan) -4. **Scan failure**: Don't update watermark on failure (retry will re-scan) -5. **New files**: Count increased → incremental scan finds them (common, fast) - -**Files to Modify**: -- `src/basic_memory/models.py` - Add last_scan_timestamp, last_file_count to Project -- `alembic/versions/xxx_add_scan_watermark.py` - Migration for new columns -- `src/basic_memory/sync/sync_service.py` - Implement watermark logic -- `src/basic_memory/repository/project_repository.py` - Update methods -- `tests/sync/test_sync_watermark.py` - Test watermark behavior - -**Test Plan**: -- [x] SSH test on TigrisFS confirms `find -newermt` works (completed) -- [x] Unit tests for scan strategy selection (4 tests) -- [x] Unit tests for file count detection (integrated in strategy tests) -- [x] Integration test: verify incremental scan finds changed files (4 tests) -- [x] Integration test: verify deletion detection triggers full scan (2 tests) -- [ ] Load test on tenant-0a20eb58 (1,460 files) - pending production deployment -- [ ] Verify <3s for no-change sync - pending production deployment - -**Implementation Status**: ✅ **COMPLETED** - -**Code Changes** (Commit: `fb16055d`): -- ✅ Added `last_scan_timestamp` and `last_file_count` to Project model -- ✅ Created database migration `e7e1f4367280_add_scan_watermark_tracking_to_project.py` -- ✅ Implemented smart scan strategy selection in `sync_service.py` -- ✅ Added `_quick_count_files()` using `find | wc -l` (~1.4s for 1,460 files) -- ✅ Added `_scan_directory_modified_since()` using `find -newermt` (~0.2s) -- ✅ Added `_scan_directory_full()` wrapper for full scans -- ✅ Watermark update logic after successful sync (uses sync START time) -- ✅ Logfire metrics for scan types and performance tracking - -**Test Coverage** (18 tests in `test_sync_service_incremental.py`): -- ✅ Scan strategy selection (4 tests) - - First sync uses full scan - - File count decreased triggers full scan - - Same file count uses incremental scan - - Increased file count uses incremental scan -- ✅ Incremental scan base cases (4 tests) - - No changes scenario - - Detects new files - - Detects modified files - - Detects multiple changes -- ✅ Deletion detection (2 tests) - - Single file deletion - - Multiple file deletions -- ✅ Move detection (2 tests) - - Moves require full scan (renames don't update mtime) - - Moves detected in full scan via checksum -- ✅ Watermark update (3 tests) - - Watermark updated after successful sync - - Watermark uses sync start time - - File count accuracy -- ✅ Edge cases (3 tests) - - Concurrent file changes - - Empty directory handling - - Respects .gitignore patterns - -**Performance Expectations** (to be verified in production): -- No changes: 420s → ~2s (210x faster) -- Few changes (5-10): 420s → ~5s (84x faster) -- Many changes (100+): 420s → ~30s (14x faster) -- Deletions: 420s → 420s (full scan, rare case) - -**Rollout Strategy**: -1. ✅ Code complete and tested (18 new tests, all passing) -2. ✅ Pushed to `phase-0.5-streaming-foundation` branch -3. ⏳ Windows CI tests running -4. 📊 Deploy to staging tenant with watermark optimization -5. 📊 Monitor scan performance metrics via Logfire -6. 📊 Verify no missed files (compare full vs incremental results) -7. 📊 Deploy to production tenant-0a20eb58 -8. 📊 Measure actual improvement (expect 420s → 2-3s) - -**Success Criteria**: -- ✅ Implementation complete with comprehensive tests -- [ ] No-change syncs complete in <3 seconds (was 420s) - pending production test -- [ ] Incremental scans (95% of cases) use watermark - pending production test -- [ ] Deletion detection works correctly (full scan when needed) - tested in unit tests ✅ -- [ ] No files missed due to watermark logic - tested in unit tests ✅ -- [ ] Metrics show scan type distribution matches expectations - pending production test - -**Next Steps**: -1. Production deployment to tenant-0a20eb58 -2. Measure actual performance improvements -3. Monitor metrics for 1 week -4. Phase 2 cloud-specific fixes -5. Phase 3 production measurement and UberSync decision - -### Phase 2: Cloud Fixes - -**Resource leaks**: -- [ ] Fix aiohttp session context manager -- [ ] Implement persistent circuit breaker -- [ ] Add memory monitoring/alerts -- [ ] Test on production tenant - -**Sync coordination**: -- [ ] Implement hash-based staggering -- [ ] Add jitter to sync intervals -- [ ] Load test with 10 concurrent tenants -- [ ] Verify no thundering herd - -### Phase 3: Measurement - -**Deploy to production**: -- [ ] Deploy Phase 1+2 changes -- [ ] Downgrade tenant-6d2ff1a3 to 1GB -- [ ] Monitor for OOM incidents - -**Collect metrics**: -- [ ] Memory usage patterns -- [ ] Sync duration distributions -- [ ] Concurrent sync load -- [ ] Cost analysis - -**UberSync decision**: -- [ ] Review metrics against decision criteria -- [ ] Document findings -- [ ] Create SPEC-18 for UberSync if needed - -## Related Issues - -### basic-memory (core) -- [#383](https://github.com/basicmachines-co/basic-memory/issues/383) - Refactor sync to use mtime-based scanning -- [#382](https://github.com/basicmachines-co/basic-memory/issues/382) - Optimize memory for large file syncs -- [#371](https://github.com/basicmachines-co/basic-memory/issues/371) - aiofiles for non-blocking I/O (future) - -### basic-memory-cloud -- [#198](https://github.com/basicmachines-co/basic-memory-cloud/issues/198) - Memory optimization for sync worker -- [#189](https://github.com/basicmachines-co/basic-memory-cloud/issues/189) - Circuit breaker for infinite retry loops - -## References - -**Standard sync tools using mtime**: -- rsync: Uses mtime-based comparison by default, only checksums on `--checksum` flag -- rclone: Default is mtime/size, `--checksum` mode optional -- syncthing: Block-level sync with mtime tracking - -**fsnotify polling** (future consideration): -- [fsnotify/fsnotify#9](https://github.com/fsnotify/fsnotify/issues/9) - Polling mode for network filesystems - -## Notes - -### Why Not UberSync Now? - -**Premature Optimization**: -- Current problems are algorithmic, not architectural -- No evidence that multi-tenant coordination is the issue -- Single tenant OOM proves algorithm is the problem - -**Benefits of Core-First Approach**: -- ✅ Helps all users (CLI + Cloud) -- ✅ Lower risk (no new service) -- ✅ Clear path (issues specify fixes) -- ✅ Can defer UberSync until proven necessary - -**When UberSync Makes Sense**: -- >100 active tenants causing resource contention -- Need for tenant tier prioritization (paid > free) -- Centralized observability requirements -- Cost optimization at scale - -### Migration Strategy - -**Backward Compatibility**: -- New mtime/size columns nullable initially -- Existing entities sync normally (compute mtime on first scan) -- No breaking changes to MCP API -- CLI behavior unchanged - -**Rollout**: -1. Deploy to staging with test tenant -2. Validate memory/performance improvements -3. Deploy to production (blue-green) -4. Monitor for 1 week -5. Downgrade tenant machines if successful - -## Further Considerations - -### Version Control System (VCS) Integration - -**Context:** Users frequently request git versioning, and large projects with PDFs/images pose memory challenges. - -#### Git-Based Sync - -**Approach:** Use git for change detection instead of custom mtime comparison. - -```python -# Git automatically tracks changes -repo = git.Repo(project_path) -repo.git.add(A=True) -diff = repo.index.diff('HEAD') - -for change in diff: - if change.change_type == 'M': # Modified - await sync_file(change.b_path) -``` - -**Pros:** -- ✅ Proven, battle-tested change detection -- ✅ Built-in rename/move detection (similarity index) -- ✅ Efficient for cloud sync (git protocol over HTTP) -- ✅ Could enable version history as bonus feature -- ✅ Users want git integration anyway - -**Cons:** -- ❌ User confusion (`.git` folder in knowledge base) -- ❌ Conflicts with existing git repos (submodule complexity) -- ❌ Adds dependency (git binary or dulwich/pygit2) -- ❌ Less control over sync logic -- ❌ Doesn't solve large file problem (PDFs still checksummed) -- ❌ Git LFS adds complexity - -#### Jujutsu (jj) Alternative - -**Why jj is compelling:** - -1. **Working Copy as Source of Truth** - - Git: Staging area is intermediate state - - Jujutsu: Working copy IS a commit - - Aligns with "files are source of truth" philosophy! - -2. **Automatic Change Tracking** - - No manual staging required - - Working copy changes tracked automatically - - Better fit for sync operations vs git's commit-centric model - -3. **Conflict Handling** - - User edits + sync changes both preserved - - Operation log vs linear history - - Built for operations, not just history - -**Cons:** -- ❌ New/immature (2020 vs git's 2005) -- ❌ Not universally available -- ❌ Steeper learning curve for users -- ❌ No LFS equivalent yet -- ❌ Still doesn't solve large file checksumming - -#### Git Index Format (Hybrid Approach) - -**Best of both worlds:** Use git's index format without full git repo. - -```python -from dulwich.index import Index # Pure Python - -# Use git index format for tracking -idx = Index(project_path / '.basic-memory' / 'index') - -for file in files: - stat = file.stat() - if idx.get(file) and idx[file].mtime == stat.st_mtime: - continue # Unchanged (git's proven logic) - - await sync_file(file) - idx[file] = (stat.st_mtime, stat.st_size, sha) -``` - -**Pros:** -- ✅ Git's proven change detection logic -- ✅ No user-visible `.git` folder -- ✅ No git dependency (pure Python) -- ✅ Full control over sync - -**Cons:** -- ❌ Adds dependency (dulwich) -- ❌ Doesn't solve large files -- ❌ No built-in versioning - -### Large File Handling - -**Problem:** PDFs/images cause memory issues regardless of VCS choice. - -**Solutions (Phase 1+):** - -**1. Skip Checksums for Large Files** -```python -if stat.st_size > 10_000_000: # 10MB threshold - checksum = None # Use mtime/size only - logger.info(f"Skipping checksum for {file_path}") -``` - -**2. Partial Hashing** -```python -if file.suffix in ['.pdf', '.jpg', '.png']: - # Hash first/last 64KB instead of entire file - checksum = hash_partial(file, chunk_size=65536) -``` - -**3. External Blob Storage** -```python -if stat.st_size > 10_000_000: - blob_id = await upload_to_tigris_blob(file) - entity.blob_id = blob_id - entity.file_path = None # Not in main sync -``` - -### Recommendation & Timeline - -**Phase 0.5-1 (Now):** Custom streaming + mtime -- ✅ Solves urgent memory issues -- ✅ No dependencies -- ✅ Full control -- ✅ Skip checksums for large files (>10MB) -- ✅ Proven pattern (rsync/rclone) - -**Phase 2 (After metrics):** Git index format exploration -```python -# Optional: Use git index for tracking if beneficial -from dulwich.index import Index -# No git repo, just index file format -``` - -**Future (User feature):** User-facing versioning -```python -# Let users opt into VCS: -basic-memory config set versioning git -basic-memory config set versioning jj -basic-memory config set versioning none # Current behavior - -# Integrate with their chosen workflow -# Not forced upon them -``` - -**Rationale:** -1. **Don't block on VCS decision** - Memory issues are P0 -2. **Learn from deployment** - See actual usage patterns -3. **Keep options open** - Can add git/jj later -4. **Files as source of truth** - Core philosophy preserved -5. **Large files need attention regardless** - VCS won't solve that - -**Decision Point:** -- If Phase 0.5/1 achieves memory targets → VCS integration deferred -- If users strongly request versioning → Add as opt-in feature -- If change detection becomes bottleneck → Explore git index format - -## Agent Assignment - -**Phase 1 Implementation**: `python-developer` agent -- Expertise in FastAPI, async Python, database migrations -- Handles basic-memory core changes - -**Phase 2 Implementation**: `python-developer` agent -- Same agent continues with cloud-specific fixes -- Maintains consistency across phases - -**Phase 3 Review**: `system-architect` agent -- Analyzes metrics and makes UberSync decision -- Creates SPEC-18 if centralized service needed diff --git a/specs/SPEC-2 Slash Commands Reference.md b/specs/SPEC-2 Slash Commands Reference.md deleted file mode 100644 index 7fb207fa..00000000 --- a/specs/SPEC-2 Slash Commands Reference.md +++ /dev/null @@ -1,120 +0,0 @@ ---- -title: 'SPEC-2: Slash Commands Reference' -type: spec -permalink: specs/spec-2-slash-commands-reference -tags: -- commands -- process -- reference ---- - -# SPEC-2: Slash Commands Reference - -This document defines the slash commands used in our specification-driven development process. - -## /spec create [name] - -**Purpose**: Create a new specification document - -**Usage**: `/spec create notes-decomposition` - -**Process**: -1. Create new spec document in `/specs` folder -2. Use SPEC-XXX numbering format (auto-increment) -3. Include standard spec template: - - Why (reasoning/problem) - - What (affected areas) - - How (high-level approach) - - How to Evaluate (testing/validation) -4. Tag appropriately for knowledge graph -5. Link to related specs/components - -**Template**: -```markdown -# SPEC-XXX: [Title] - -## Why -[Problem statement and reasoning] - -## What -[What is affected or changed] - -## How (High Level) -[Approach to implementation] - -## How to Evaluate -[Testing/validation procedure] - -## Notes -[Additional context as needed] -``` - -## /spec status - -**Purpose**: Show current status of all specifications - -**Usage**: `/spec status` - -**Process**: -1. Search all specs in `/specs` folder -2. Display table showing: - - Spec number and title - - Status (draft, approved, implementing, complete) - - Assigned agent (if any) - - Last updated - - Dependencies - -## /spec implement [name] - -**Purpose**: Hand specification to appropriate agent for implementation - -**Usage**: `/spec implement SPEC-002` - -**Process**: -1. Read the specified spec -2. Analyze requirements to determine appropriate agent: - - Frontend components → vue-developer - - Architecture/system design → system-architect - - Backend/API → python-developer -3. Launch agent with spec context -4. Agent creates implementation plan -5. Update spec with implementation status - -## /spec review [name] - -**Purpose**: Review implementation against specification criteria - -**Usage**: `/spec review SPEC-002` - -**Process**: -1. Read original spec and "How to Evaluate" section -2. Examine current implementation -3. Test against success criteria -4. Document gaps or issues -5. Update spec with review results -6. Recommend next actions (complete, revise, iterate) - -## Command Extensions - -As the process evolves, we may add: -- `/spec link [spec1] [spec2]` - Create dependency links -- `/spec archive [name]` - Archive completed specs -- `/spec template [type]` - Create spec from template -- `/spec search [query]` - Search spec content - -## References - -- Claude Slash commands: https://docs.anthropic.com/en/docs/claude-code/slash-commands - -## Creating a command - -Commands are implemented as Claude slash commands: - -Location in repo: .claude/commands/ - -In the following example, we create the /optimize command: -```bash -# Create a project command -mkdir -p .claude/commands -echo "Analyze this code for performance issues and suggest optimizations:" > .claude/commands/optimize.md -``` diff --git a/specs/SPEC-20 Simplified Project-Scoped Rclone Sync.md b/specs/SPEC-20 Simplified Project-Scoped Rclone Sync.md deleted file mode 100644 index 65055f34..00000000 --- a/specs/SPEC-20 Simplified Project-Scoped Rclone Sync.md +++ /dev/null @@ -1,1216 +0,0 @@ ---- -title: 'SPEC-20: Simplified Project-Scoped Rclone Sync' -date: 2025-01-27 -updated: 2025-01-28 -status: Implemented -priority: High -goal: Simplify cloud sync by making it project-scoped, safe by design, and closer to native rclone commands -parent: SPEC-8 ---- - -## Executive Summary - -The current rclone implementation (SPEC-8) has proven too complex with multiple footguns: -- Two workflows (mount vs bisync) with different directories causing confusion -- Multiple profiles (3 for mount, 3 for bisync) creating too much choice -- Directory conflicts (`~/basic-memory-cloud/` vs `~/basic-memory-cloud-sync/`) -- Auto-discovery of folders leading to errors -- Unclear what syncs and when - -This spec proposes a **radical simplification**: project-scoped sync operations that are explicit, safe, and thin wrappers around rclone commands. - -## Why - -### Current Problems - -**Complexity:** -- Users must choose between mount and bisync workflows -- Different directories for different workflows -- Profile selection (6 total profiles) overwhelms users -- Setup requires multiple steps with potential conflicts - -**Footguns:** -- Renaming local folder breaks sync (no config tracking) -- Mount directory conflicts with bisync directory -- Auto-discovered folders create phantom projects -- Uninitialized bisync state causes failures -- Unclear which files sync (all files in root directory?) - -**User Confusion:** -- "What does `bm sync` actually do?" -- "Is `~/basic-memory-cloud-sync/my-folder/` a project or just a folder?" -- "Why do I have two basic-memory directories?" -- "How do I sync just one project?" - -### Design Principles (Revised) - -1. **Projects are independent** - Each project manages its own sync state -2. **Global cloud mode** - You're either local or cloud (no per-project flag) -3. **Explicit operations** - No auto-discovery, no magic -4. **Safe by design** - Config tracks state, not filesystem -5. **Thin rclone wrappers** - Stay close to rclone commands -6. **One good way** - Remove choices that don't matter - -## What - -### New Architecture - -#### Core Concepts - -1. **Global Cloud Mode** (existing, keep as-is) - - `cloud_mode_enabled` in config - - `bm cloud login` enables it, `bm cloud logout` disables it - - When enabled, ALL Basic Memory operations hit cloud API - -2. **Project-Scoped Sync** (new) - - Each project optionally has a `local_path` for local working copy - - Sync operations are explicit: `bm project sync --name research` - - Projects can live anywhere on disk, not forced into sync directory - -3. **Simplified rclone Config** (new) - - Single remote named `bm-cloud` (not `basic-memory-{tenant_id}`) - - One credential set per user - - Config lives at `~/.config/rclone/rclone.conf` - -#### Command Structure - -```bash -# Setup (once) -bm cloud login # Authenticate, enable cloud mode -bm cloud setup # Install rclone, generate credentials - -# Project creation with optional sync -bm project add research # Create cloud project (no local sync) -bm project add research --local ~/docs # Create with local sync configured - -# Or configure sync later -bm project sync-setup research ~/docs # Configure local sync for existing project - -# Project-scoped rclone operations -bm project sync --name research # One-way sync (local → cloud) -bm project bisync --name research # Two-way sync (local ↔ cloud) -bm project check --name research # Verify integrity - -# Advanced file operations -bm project ls --name research [path] # List remote files -bm project copy --name research src dst # Copy files - -# Batch operations -bm project sync --all # Sync all projects with local_sync_path -bm project bisync --all # Two-way sync all projects -``` - -#### Config Model - -```json -{ - "cloud_mode": true, - "cloud_host": "https://cloud.basicmemory.com", - - "projects": { - // Used in LOCAL mode only (simple name → path mapping) - "main": "/Users/user/basic-memory" - }, - - "cloud_projects": { - // Used in CLOUD mode for sync configuration - "research": { - "local_path": "~/Documents/research", - "last_sync": "2025-01-27T10:00:00Z", - "bisync_initialized": true - }, - "work": { - "local_path": "~/work", - "last_sync": null, - "bisync_initialized": false - } - } -} -``` - -**Note:** In cloud mode, the actual project list comes from the API (`GET /projects/projects`). The `cloud_projects` dict only stores local sync configuration. - -#### Rclone Config - -```ini -# ~/.config/rclone/rclone.conf -[basic-memory-cloud] -type = s3 -provider = Other -access_key_id = {scoped_access_key} -secret_access_key = {scoped_secret_key} -endpoint = https://fly.storage.tigris.dev -region = auto -``` - -### What Gets Removed - -- ❌ Mount commands (`bm cloud mount`, `bm cloud unmount`, `bm cloud mount-status`) -- ❌ Profile selection (both mount and bisync profiles) -- ❌ `~/basic-memory-cloud/` directory (mount point) -- ❌ `~/basic-memory-cloud-sync/` directory (forced sync location) -- ❌ Auto-discovery of folders -- ❌ Separate `bisync-setup` command -- ❌ `bisync_config` in config.json -- ❌ Convenience commands (`bm sync`, `bm bisync` without project name) -- ❌ Complex state management for global sync - -### What Gets Simplified - -- ✅ One setup command: `bm cloud setup` -- ✅ One rclone remote: `bm-cloud` -- ✅ One workflow: project-scoped bisync (remove mount) -- ✅ One set of defaults (balanced settings from SPEC-8) -- ✅ Clear project-to-path mapping in config -- ✅ Explicit sync operations only - -### What Gets Added - -- ✅ `bm project sync --name ` (one-way: local → cloud) -- ✅ `bm project bisync --name ` (two-way: local ↔ cloud) -- ✅ `bm project check --name ` (integrity verification) -- ✅ `bm project sync-setup ` (configure sync) -- ✅ `bm project ls --name [path]` (list remote files) -- ✅ `bm project copy --name ` (copy files) -- ✅ `cloud_projects` dict in config.json - -## How - -### Phase 1: Project Model Updates - -**1.1 Update Config Schema** - -```python -# basic_memory/config.py -class Config(BaseModel): - # ... existing fields ... - cloud_mode: bool = False - cloud_host: str = "https://cloud.basicmemory.com" - - # Local mode: simple name → path mapping - projects: dict[str, str] = {} - - # Cloud mode: sync configuration per project - cloud_projects: dict[str, CloudProjectConfig] = {} - - -class CloudProjectConfig(BaseModel): - """Sync configuration for a cloud project.""" - local_path: str # Local working directory - last_sync: Optional[datetime] = None # Last successful sync - bisync_initialized: bool = False # Whether bisync baseline exists -``` - -**No database changes needed** - sync config lives in `~/.basic-memory/config.json` only. - -### Phase 2: Simplified Rclone Configuration - -**2.1 Simplify Remote Naming** - -```python -# basic_memory/cli/commands/cloud/rclone_config.py - -def configure_rclone_remote( - access_key: str, - secret_key: str, - endpoint: str = "https://fly.storage.tigris.dev", - region: str = "auto", -) -> None: - """Configure single rclone remote named 'bm-cloud'.""" - - config = load_rclone_config() - - # Single remote name (not tenant-specific) - REMOTE_NAME = "basic-memory-cloud" - - if not config.has_section(REMOTE_NAME): - config.add_section(REMOTE_NAME) - - config.set(REMOTE_NAME, "type", "s3") - config.set(REMOTE_NAME, "provider", "Other") - config.set(REMOTE_NAME, "access_key_id", access_key) - config.set(REMOTE_NAME, "secret_access_key", secret_key) - config.set(REMOTE_NAME, "endpoint", endpoint) - config.set(REMOTE_NAME, "region", region) - - save_rclone_config(config) -``` - -**2.2 Remove Profile Complexity** - -Use single set of balanced defaults (from SPEC-8 Phase 4 testing): -- `conflict_resolve`: `newer` (auto-resolve to most recent) -- `max_delete`: `25` (safety limit) -- `check_access`: `false` (skip for performance) - -### Phase 3: Project-Scoped Rclone Commands - -**3.1 Core Sync Operations** - -```python -# basic_memory/cli/commands/cloud/rclone_commands.py - -def get_project_remote(project: Project, bucket_name: str) -> str: - """Build rclone remote path for project. - - Returns: bm-cloud:bucket-name/app/data/research - """ - # Strip leading slash from cloud path - cloud_path = project.path.lstrip("/") - return f"basic-memory-cloud:{bucket_name}/{cloud_path}" - - -def project_sync( - project: Project, - bucket_name: str, - dry_run: bool = False, - verbose: bool = False, -) -> bool: - """One-way sync: local → cloud. - - Uses rclone sync to make cloud identical to local. - """ - if not project.local_sync_path: - raise RcloneError(f"Project {project.name} has no local_sync_path configured") - - local_path = Path(project.local_sync_path).expanduser() - remote_path = get_project_remote(project, bucket_name) - filter_path = get_bmignore_filter_path() - - cmd = [ - "rclone", "sync", - str(local_path), - remote_path, - "--filters-file", str(filter_path), - ] - - if verbose: - cmd.append("--verbose") - else: - cmd.append("--progress") - - if dry_run: - cmd.append("--dry-run") - - result = subprocess.run(cmd, text=True) - return result.returncode == 0 - - -def project_bisync( - project: Project, - bucket_name: str, - dry_run: bool = False, - resync: bool = False, - verbose: bool = False, -) -> bool: - """Two-way sync: local ↔ cloud. - - Uses rclone bisync with balanced defaults. - """ - if not project.local_sync_path: - raise RcloneError(f"Project {project.name} has no local_sync_path configured") - - local_path = Path(project.local_sync_path).expanduser() - remote_path = get_project_remote(project, bucket_name) - filter_path = get_bmignore_filter_path() - state_path = get_project_bisync_state(project.name) - - # Ensure state directory exists - state_path.mkdir(parents=True, exist_ok=True) - - cmd = [ - "rclone", "bisync", - str(local_path), - remote_path, - "--create-empty-src-dirs", - "--resilient", - "--conflict-resolve=newer", - "--max-delete=25", - "--filters-file", str(filter_path), - "--workdir", str(state_path), - ] - - if verbose: - cmd.append("--verbose") - else: - cmd.append("--progress") - - if dry_run: - cmd.append("--dry-run") - - if resync: - cmd.append("--resync") - - # Check if first run requires resync - if not resync and not bisync_initialized(project.name) and not dry_run: - raise RcloneError( - f"First bisync for {project.name} requires --resync to establish baseline.\n" - f"Run: bm project bisync --name {project.name} --resync" - ) - - result = subprocess.run(cmd, text=True) - return result.returncode == 0 - - -def project_check( - project: Project, - bucket_name: str, - one_way: bool = False, -) -> bool: - """Check integrity between local and cloud. - - Returns True if files match, False if differences found. - """ - if not project.local_sync_path: - raise RcloneError(f"Project {project.name} has no local_sync_path configured") - - local_path = Path(project.local_sync_path).expanduser() - remote_path = get_project_remote(project, bucket_name) - filter_path = get_bmignore_filter_path() - - cmd = [ - "rclone", "check", - str(local_path), - remote_path, - "--filter-from", str(filter_path), - ] - - if one_way: - cmd.append("--one-way") - - result = subprocess.run(cmd, capture_output=True, text=True) - return result.returncode == 0 -``` - -**3.2 Advanced File Operations** - -```python -def project_ls( - project: Project, - bucket_name: str, - path: Optional[str] = None, -) -> list[str]: - """List files in remote project.""" - remote_path = get_project_remote(project, bucket_name) - if path: - remote_path = f"{remote_path}/{path}" - - cmd = ["rclone", "ls", remote_path] - result = subprocess.run(cmd, capture_output=True, text=True, check=True) - return result.stdout.splitlines() - - -def project_copy( - project: Project, - bucket_name: str, - src: str, - dst: str, - dry_run: bool = False, -) -> bool: - """Copy files within project scope.""" - # Implementation similar to sync - pass -``` - -### Phase 4: CLI Integration - -**4.1 Update Project Commands** - -```python -# basic_memory/cli/commands/project.py - -@project_app.command("add") -def add_project( - name: str = typer.Argument(..., help="Name of the project"), - path: str = typer.Argument(None, help="Path (required for local mode)"), - local: str = typer.Option(None, "--local", help="Local sync path for cloud mode"), - set_default: bool = typer.Option(False, "--default", help="Set as default"), -) -> None: - """Add a new project. - - Cloud mode examples: - bm project add research # No local sync - bm project add research --local ~/docs # With local sync - - Local mode example: - bm project add research ~/Documents/research - """ - config = ConfigManager().config - - if config.cloud_mode_enabled: - # Cloud mode: auto-generate cloud path from name - async def _add_project(): - async with get_client() as client: - data = { - "name": name, - "path": generate_permalink(name), - "local_sync_path": local, # Optional - "set_default": set_default, - } - response = await call_post(client, "/projects/projects", json=data) - return ProjectStatusResponse.model_validate(response.json()) - else: - # Local mode: path is required - if path is None: - console.print("[red]Error: path argument is required in local mode[/red]") - raise typer.Exit(1) - - resolved_path = Path(os.path.abspath(os.path.expanduser(path))).as_posix() - - async def _add_project(): - async with get_client() as client: - data = { - "name": name, - "path": resolved_path, - "set_default": set_default, - } - response = await call_post(client, "/projects/projects", json=data) - return ProjectStatusResponse.model_validate(response.json()) - - # Execute and display result - result = asyncio.run(_add_project()) - console.print(f"[green]{result.message}[/green]") - - -@project_app.command("sync-setup") -def setup_project_sync( - name: str = typer.Argument(..., help="Project name"), - local_path: str = typer.Argument(..., help="Local sync directory"), -) -> None: - """Configure local sync for an existing cloud project.""" - - config = ConfigManager().config - - if not config.cloud_mode_enabled: - console.print("[red]Error: sync-setup only available in cloud mode[/red]") - raise typer.Exit(1) - - resolved_path = Path(os.path.abspath(os.path.expanduser(local_path))).as_posix() - - async def _update_project(): - async with get_client() as client: - data = {"local_sync_path": resolved_path} - project_permalink = generate_permalink(name) - response = await call_patch( - client, - f"/projects/{project_permalink}", - json=data, - ) - return ProjectStatusResponse.model_validate(response.json()) - - result = asyncio.run(_update_project()) - console.print(f"[green]{result.message}[/green]") - console.print(f"\nLocal sync configured: {resolved_path}") - console.print(f"\nTo sync: bm project bisync --name {name} --resync") -``` - -**4.2 New Sync Commands** - -```python -# basic_memory/cli/commands/project.py - -@project_app.command("sync") -def sync_project( - name: str = typer.Option(..., "--name", help="Project name"), - all_projects: bool = typer.Option(False, "--all", help="Sync all projects"), - dry_run: bool = typer.Option(False, "--dry-run", help="Preview changes"), - verbose: bool = typer.Option(False, "--verbose", help="Show detailed output"), -) -> None: - """One-way sync: local → cloud (make cloud identical to local).""" - - config = ConfigManager().config - if not config.cloud_mode_enabled: - console.print("[red]Error: sync only available in cloud mode[/red]") - raise typer.Exit(1) - - # Get projects to sync - if all_projects: - projects = get_all_sync_projects() - else: - projects = [get_project_by_name(name)] - - # Get bucket name - tenant_info = asyncio.run(get_mount_info()) - bucket_name = tenant_info.bucket_name - - # Sync each project - for project in projects: - if not project.local_sync_path: - console.print(f"[yellow]Skipping {project.name}: no local_sync_path[/yellow]") - continue - - console.print(f"[blue]Syncing {project.name}...[/blue]") - try: - project_sync(project, bucket_name, dry_run=dry_run, verbose=verbose) - console.print(f"[green]✓ {project.name} synced[/green]") - except RcloneError as e: - console.print(f"[red]✗ {project.name} failed: {e}[/red]") - - -@project_app.command("bisync") -def bisync_project( - name: str = typer.Option(..., "--name", help="Project name"), - all_projects: bool = typer.Option(False, "--all", help="Bisync all projects"), - dry_run: bool = typer.Option(False, "--dry-run", help="Preview changes"), - resync: bool = typer.Option(False, "--resync", help="Force new baseline"), - verbose: bool = typer.Option(False, "--verbose", help="Show detailed output"), -) -> None: - """Two-way sync: local ↔ cloud (bidirectional sync).""" - - # Similar to sync but calls project_bisync() - pass - - -@project_app.command("check") -def check_project( - name: str = typer.Option(..., "--name", help="Project name"), - one_way: bool = typer.Option(False, "--one-way", help="Check one direction only"), -) -> None: - """Verify file integrity between local and cloud.""" - - # Calls project_check() - pass - - -@project_app.command("ls") -def list_project_files( - name: str = typer.Option(..., "--name", help="Project name"), - path: str = typer.Argument(None, help="Path within project"), -) -> None: - """List files in remote project.""" - - # Calls project_ls() - pass -``` - -**4.3 Update Cloud Setup** - -```python -# basic_memory/cli/commands/cloud/core_commands.py - -@cloud_app.command("setup") -def cloud_setup() -> None: - """Set up cloud sync (install rclone and configure credentials).""" - - console.print("[bold blue]Basic Memory Cloud Setup[/bold blue]") - console.print("Installing rclone and configuring credentials...\n") - - try: - # Step 1: Install rclone - console.print("[blue]Step 1: Installing rclone...[/blue]") - install_rclone() - - # Step 2: Get tenant info - console.print("\n[blue]Step 2: Getting tenant information...[/blue]") - tenant_info = asyncio.run(get_mount_info()) - tenant_id = tenant_info.tenant_id - - console.print(f"[green]✓ Tenant: {tenant_id}[/green]") - - # Step 3: Generate credentials - console.print("\n[blue]Step 3: Generating sync credentials...[/blue]") - creds = asyncio.run(generate_mount_credentials(tenant_id)) - - console.print("[green]✓ Generated credentials[/green]") - - # Step 4: Configure rclone (single remote: bm-cloud) - console.print("\n[blue]Step 4: Configuring rclone...[/blue]") - configure_rclone_remote( - access_key=creds.access_key, - secret_key=creds.secret_key, - ) - - console.print("\n[bold green]✓ Cloud setup completed![/bold green]") - console.print("\nNext steps:") - console.print(" 1. Create projects with local sync:") - console.print(" bm project add research --local ~/Documents/research") - console.print("\n 2. Or configure sync for existing projects:") - console.print(" bm project sync-setup research ~/Documents/research") - console.print("\n 3. Start syncing:") - console.print(" bm project bisync --name research --resync") - - except Exception as e: - console.print(f"\n[red]Setup failed: {e}[/red]") - raise typer.Exit(1) -``` - -### Phase 5: Cleanup - -**5.1 Remove Deprecated Commands** - -```python -# Remove from cloud commands: -- cloud mount -- cloud unmount -- cloud mount-status -- bisync-setup -- Individual bisync command (moved to project bisync) - -# Remove from root commands: -- bm sync (without project specification) -- bm bisync (without project specification) -``` - -**5.2 Remove Deprecated Code** - -```python -# Files to remove: -- mount_commands.py (entire file) - -# Functions to remove from rclone_config.py: -- MOUNT_PROFILES -- get_default_mount_path() -- build_mount_command() -- is_path_mounted() -- get_rclone_processes() -- kill_rclone_process() -- unmount_path() -- cleanup_orphaned_rclone_processes() - -# Functions to remove from bisync_commands.py: -- BISYNC_PROFILES (use single default) -- setup_cloud_bisync() (replaced by cloud setup) -- run_bisync_watch() (can add back to project bisync if needed) -- show_bisync_status() (replaced by project list showing sync status) -``` - -**5.3 Update Configuration Schema** - -```python -# Remove from config.json: -- bisync_config (no longer needed) - -# The projects array is the source of truth for sync configuration -``` - -### Phase 6: Documentation Updates - -**6.1 Update CLI Documentation** - -```markdown -# docs/cloud-cli.md - -## Project-Scoped Cloud Sync - -Basic Memory cloud sync is project-scoped - each project can optionally be configured with a local working directory that syncs with the cloud. - -### Setup (One Time) - -1. Authenticate and enable cloud mode: - ```bash - bm cloud login - ``` - -2. Install rclone and configure credentials: - ```bash - bm cloud setup - ``` - -### Create Projects with Sync - -Create a cloud project with optional local sync: - -```bash -# Create project without local sync -bm project add research - -# Create project with local sync -bm project add research --local ~/Documents/research -``` - -Or configure sync for existing (remote) project: - -```bash -bm project sync-setup research ~/Documents/research -``` - -### Syncing Projects - -**Two-way sync (recommended):** -```bash -# First time - establish baseline -bm project bisync --name research --resync - -# Subsequent syncs -bm project bisync --name research - -# Sync all projects with local_sync_path configured -bm project bisync --all -``` - -**One-way sync (local → cloud):** -```bash -bm project sync --name research -``` - -**Verify integrity:** -```bash -bm project check --name research -``` - -### Advanced Operations - -**List remote files:** -```bash -bm project ls --name research -bm project ls --name research subfolder -``` - -**Preview changes before syncing:** -```bash -bm project bisync --name research --dry-run -``` - -**Verbose output for debugging:** -```bash -bm project bisync --name research --verbose -``` - -### Project Management - -**List projects (shows sync status):** -```bash -bm project list -``` - -**Update sync path:** -```bash -bm project sync-setup research ~/new/path -``` - -**Remove project:** -```bash -bm project remove research -``` -``` - -**6.2 Update SPEC-8** - -Add to SPEC-8's "Implementation Notes" section: - -```markdown -## Superseded by SPEC-20 - -The initial implementation in SPEC-8 proved too complex with multiple footguns: -- Mount vs bisync workflow confusion -- Multiple profiles creating decision paralysis -- Directory conflicts and auto-discovery errors - -SPEC-20 supersedes the sync implementation with a simplified project-scoped approach while keeping the core Tigris infrastructure from SPEC-8. -``` - -## How to Evaluate - -### Success Criteria - -**1. Simplified Setup** -- [ ] `bm cloud setup` completes in one command -- [ ] Creates single rclone remote named `bm-cloud` -- [ ] No profile selection required -- [ ] Clear next steps printed after setup - -**2. Clear Project Model** -- [ ] Projects can be created with or without local sync -- [ ] `bm project list` shows sync status for each project -- [ ] `local_sync_path` stored in project config -- [ ] Renaming local folder doesn't break sync (config is source of truth) - -**3. Working Sync Operations** -- [ ] `bm project sync --name ` performs one-way sync -- [ ] `bm project bisync --name ` performs two-way sync -- [ ] `bm project check --name ` verifies integrity -- [ ] `--all` flag syncs all configured projects -- [ ] `--dry-run` shows changes without applying -- [ ] First bisync requires `--resync` with clear error message - -**4. Safety** -- [ ] Cannot sync project without `local_sync_path` configured -- [ ] Bisync state is per-project (not global) -- [ ] `.bmignore` patterns respected -- [ ] Max delete safety (25 files) prevents accidents -- [ ] Clear error messages for all failure modes - -**5. Clean Removal** -- [ ] Mount commands removed -- [ ] Profile selection removed -- [ ] Global sync directory removed (`~/basic-memory-cloud-sync/`) -- [ ] Auto-discovery removed -- [ ] Convenience commands (`bm sync`) removed - -**6. Documentation** -- [ ] Updated cloud-cli.md with new workflow -- [ ] Clear examples for common operations -- [ ] Migration guide for existing users -- [ ] Troubleshooting section - -### Test Scenarios - -**Scenario 1: New User Setup** -```bash -# Start fresh -bm cloud login -bm cloud setup -bm project add research --local ~/docs/research -bm project bisync --name research --resync -# Edit files locally -bm project bisync --name research -# Verify changes synced -``` - -**Scenario 2: Multiple Projects** -```bash -bm project add work --local ~/work -bm project add personal --local ~/personal -bm project bisync --all --resync -# Edit files in both projects -bm project bisync --all -``` - -**Scenario 3: Project Without Sync** -```bash -bm project add temp-notes -# Try to sync (should fail gracefully) -bm project bisync --name temp-notes -# Should see: "Project temp-notes has no local_sync_path configured" -``` - -**Scenario 4: Integrity Check** -```bash -bm project bisync --name research -# Manually edit file in cloud UI -bm project check --name research -# Should report differences -bm project bisync --name research -# Should sync changes back to local -``` - -**Scenario 5: Safety Features** -```bash -# Delete 30 files locally -bm project sync --name research -# Should fail with max delete error -# User reviews and confirms -bm project sync --name research # After confirming -``` - -### Performance Targets - -- Setup completes in < 30 seconds -- Single project sync < 5 seconds for small changes -- Bisync initialization (--resync) < 10 seconds for typical project -- Batch sync (--all) processes N projects in N*5 seconds - -### Breaking Changes - -This is a **breaking change** from SPEC-8 implementation: - -**Migration Required:** -- Users must run `bm cloud setup` again -- Existing `~/basic-memory-cloud-sync/` directory abandoned -- Projects must be configured with `local_sync_path` -- Mount users must switch to bisync workflow - -**Migration Guide:** -```bash -# 1. Note current project locations -bm project list - -# 2. Re-run setup -bm cloud setup - -# 3. Configure sync for each project -bm project sync-setup research ~/Documents/research -bm project sync-setup work ~/work - -# 4. Establish baselines -bm project bisync --all --resync - -# 5. Old directory can be deleted -rm -rf ~/basic-memory-cloud-sync/ -``` - -## Dependencies - -- **SPEC-8**: TigrisFS Integration (bucket provisioning, credentials) -- Python 3.12+ -- rclone 1.64.0+ -- Typer (CLI framework) -- Rich (console output) - -## Risks - -**Risk 1: User Confusion from Breaking Changes** -- Mitigation: Clear migration guide, version bump (0.16.0) -- Mitigation: Detect old config and print migration instructions - -**Risk 2: Per-Project Bisync State Complexity** -- Mitigation: Use rclone's `--workdir` to isolate state per project -- Mitigation: Store in `~/.basic-memory/bisync-state/{project_name}/` - -**Risk 3: Batch Operations Performance** -- Mitigation: Run syncs sequentially with progress indicators -- Mitigation: Add `--parallel` flag in future if needed - -**Risk 4: Lost Features (Mount)** -- Mitigation: Document mount as experimental/advanced feature -- Mitigation: Can restore if users request it - -## Open Questions - -1. **Should we keep mount as experimental command?** - - Lean toward: Remove entirely, focus on bisync - - Alternative: Keep as `bm project mount --name ` (advanced) - - - Answer: remove - -2. **Batch sync order?** - - Alphabetical by project name? - - By last modified time? - - Let user specify order? - - answer: project order from api or config - -3. **Credential refresh?** - - Auto-detect expired credentials and re-run credential generation? - - Or require manual `bm cloud setup` again? - - answer: manual setup is fine - -4. **Watch mode for projects?** - - `bm project bisync --name research --watch`? - - Or removed entirely (users can use OS tools)? - - answer: remove for now: we can add it back later if it's useful - -5. **Project path validation?** - - Ensure `local_path` exists before allowing bisync? - - Or let rclone error naturally? - - answer: create if needed, exists is ok - -## Implementation Checklist - -### Phase 1: Config Schema (1-2 days) ✅ -- [x] Add `CloudProjectConfig` model to `basic_memory/config.py` -- [x] Add `cloud_projects: dict[str, CloudProjectConfig]` to Config model -- [x] Test config loading/saving with new schema -- [x] Handle migration from old config format - -### Phase 2: Rclone Config Simplification ✅ -- [x] Update `configure_rclone_remote()` to use `basic-memory-cloud` as remote name -- [x] Remove `add_tenant_to_rclone_config()` (replaced by configure_rclone_remote) -- [x] Remove tenant_id from remote naming -- [x] Test rclone config generation -- [x] Clean up deprecated import references in bisync_commands.py and core_commands.py - -### Phase 3: Project-Scoped Rclone Commands ✅ -- [x] Create `src/basic_memory/cli/commands/cloud/rclone_commands.py` -- [x] Implement `get_project_remote(project, bucket_name)` -- [x] Implement `project_sync()` (one-way: local → cloud) -- [x] Implement `project_bisync()` (two-way: local ↔ cloud) -- [x] Implement `project_check()` (integrity verification) -- [x] Implement `project_ls()` (list remote files) -- [x] Add helper: `get_project_bisync_state(project_name)` -- [x] Add helper: `bisync_initialized(project_name)` -- [x] Add helper: `get_bmignore_filter_path()` -- [x] Add `SyncProject` dataclass for project representation -- [x] Write unit tests for rclone commands (22 tests, 99% coverage) -- [x] Temporarily disable mount commands in core_commands.py - -### Phase 4: CLI Integration ✅ -- [x] Update `project.py`: Add `--local-path` flag to `project add` command -- [x] Update `project.py`: Create `project sync-setup` command -- [x] Create `project.py`: Add `project sync` command -- [x] Create `project.py`: Add `project bisync` command -- [x] Create `project.py`: Add `project check` command -- [x] Create `project.py`: Add `project ls` command -- [x] Create `project.py`: Add `project bisync-reset` command -- [x] Import rclone_commands module and get_mount_info helper -- [x] Update `project list` to show local sync paths in cloud mode -- [x] Update `project list` to conditionally show columns based on config -- [x] Update `project remove` to clean up local directories and bisync state -- [x] Add automatic database sync trigger after file sync operations -- [x] Add path normalization to prevent S3 mount point leakage -- [x] Update `cloud/core_commands.py`: Simplified `cloud setup` command -- [x] Write unit tests for `project add --local-path` (4 tests passing) - -### Phase 5: Cleanup ✅ -- [x] Remove `mount_commands.py` (entire file) -- [x] Remove mount-related functions from `rclone_config.py`: - - [x] `MOUNT_PROFILES` - - [x] `get_default_mount_path()` - - [x] `build_mount_command()` - - [x] `is_path_mounted()` - - [x] `get_rclone_processes()` - - [x] `kill_rclone_process()` - - [x] `unmount_path()` - - [x] `cleanup_orphaned_rclone_processes()` -- [x] Remove from `bisync_commands.py`: - - [x] `BISYNC_PROFILES` (use single default) - - [x] `setup_cloud_bisync()` - - [x] `run_bisync_watch()` - - [x] `show_bisync_status()` - - [x] `run_bisync()` - - [x] `run_check()` -- [x] Remove `bisync_config` from config schema -- [x] Remove deprecated cloud commands: - - [x] `cloud mount` - - [x] `cloud unmount` - - [x] Simplified `cloud setup` to just install rclone and configure credentials -- [x] Remove convenience commands: - - [x] Root-level `bm sync` (removed - confusing in cloud mode, automatic in local mode) -- [x] Update tests to remove references to deprecated functionality -- [x] All typecheck errors resolved - -### Phase 6: Documentation ✅ -- [x] Update `docs/cloud-cli.md` with new workflow -- [x] Add troubleshooting section for empty directory issues -- [x] Add troubleshooting section for bisync state corruption -- [x] Document `bisync-reset` command usage -- [x] Update command reference with all new commands -- [x] Add examples for common workflows -- [ ] Add migration guide for existing users (deferred - no users on old system yet) -- [ ] Update SPEC-8 with "Superseded by SPEC-20" note (deferred) - -### Testing & Validation ✅ -- [x] Test Scenario 1: New user setup (manual testing complete) -- [x] Test Scenario 2: Multiple projects (manual testing complete) -- [x] Test Scenario 3: Project without sync (manual testing complete) -- [x] Test Scenario 4: Integrity check (manual testing complete) -- [x] Test Scenario 5: bisync-reset command (manual testing complete) -- [x] Test cleanup on remove (manual testing complete) -- [x] Verify all commands work end-to-end -- [x] Document known issues (empty directory bisync limitation) -- [ ] Automated integration tests (deferred) -- [ ] Test migration from SPEC-8 implementation (N/A - no users yet) - -## Implementation Notes - -### Key Improvements Added During Implementation - -**1. Path Normalization (Critical Bug Fix)** - -**Problem:** Files were syncing to `/app/data/app/data/project/` instead of `/app/data/project/` - -**Root cause:** -- S3 bucket contains projects directly (e.g., `basic-memory-llc/`) -- Fly machine mounts bucket at `/app/data/` -- API returns paths like `/app/data/basic-memory-llc` (mount point + project) -- Rclone was using this full path, causing path doubling - -**Solution (three layers):** -- API side: Added `normalize_project_path()` in `project_router.py` to strip `/app/data/` prefix -- CLI side: Added defensive normalization in `project.py` commands -- Rclone side: Updated `get_project_remote()` to strip prefix before building remote path - -**Files modified:** -- `src/basic_memory/api/routers/project_router.py` - API normalization -- `src/basic_memory/cli/commands/project.py` - CLI normalization -- `src/basic_memory/cli/commands/cloud/rclone_commands.py` - Rclone remote path construction - -**2. Automatic Database Sync After File Operations** - -**Enhancement:** After successful file sync or bisync, automatically trigger database sync via API - -**Implementation:** -- After `project sync`: POST to `/{project}/project/sync` -- After `project bisync`: POST to `/{project}/project/sync` + update config timestamps -- Skip trigger on `--dry-run` -- Graceful error handling with warnings - -**Benefit:** Files and database stay in sync automatically without manual intervention - -**3. Enhanced Project Removal with Cleanup** - -**Enhancement:** `bm project remove` now properly cleans up local artifacts - -**Behavior with `--delete-notes`:** -- ✓ Removes project from cloud API -- ✓ Deletes cloud files -- ✓ Removes local sync directory -- ✓ Removes bisync state directory -- ✓ Removes `cloud_projects` config entry - -**Behavior without `--delete-notes`:** -- ✓ Removes project from cloud API -- ✗ Keeps local files (shows path in message) -- ✓ Removes bisync state directory (cleanup) -- ✓ Removes `cloud_projects` config entry - -**Files modified:** -- `src/basic_memory/cli/commands/project.py` - Enhanced `remove_project()` function - -**4. Bisync State Reset Command** - -**New command:** `bm project bisync-reset ` - -**Purpose:** Clear bisync state when it becomes corrupted (e.g., after mixing dry-run and actual runs) - -**What it does:** -- Removes all bisync metadata from `~/.basic-memory/bisync-state/{project}/` -- Forces fresh baseline on next `--resync` -- Safe operation (doesn't touch files) -- Also runs automatically on project removal - -**Files created:** -- Added `bisync-reset` command to `src/basic_memory/cli/commands/project.py` - -**5. Improved UI for Project List** - -**Enhancements:** -- Shows "Local Path" column in cloud mode for projects with sync configured -- Conditionally shows/hides columns based on config: - - Local Path: only in cloud mode - - Default: only when `default_project_mode` is True -- Uses `no_wrap=True, overflow="fold"` to prevent path truncation -- Applies path normalization to prevent showing mount point details - -**Files modified:** -- `src/basic_memory/cli/commands/project.py` - Enhanced `list_projects()` function - -**6. Documentation of Known Issues** - -**Issue documented:** Rclone bisync limitation with empty directories - -**Problem:** "Empty prior Path1 listing. Cannot sync to an empty directory" - -**Explanation:** Bisync creates listing files that track state. When both directories are completely empty, these listing files are considered invalid. - -**Solution documented:** Add at least one file (like README.md) before running `--resync` - -**Files updated:** -- `docs/cloud-cli.md` - Added troubleshooting sections for: - - Empty directory issues - - Bisync state corruption - - Usage of `bisync-reset` command - -### Rclone Flag Fix - -**Bug fix:** Incorrect rclone flag causing sync failures - -**Error:** `unknown flag: --filters-file` - -**Fix:** Changed `--filters-file` to correct flag `--filter-from` in both `project_sync()` and `project_bisync()` functions - -**Files modified:** -- `src/basic_memory/cli/commands/cloud/rclone_commands.py` - -### Test Coverage - -**Unit tests added:** -- `tests/cli/test_project_add_with_local_path.py` - 4 tests for `--local-path` functionality - - Test with local path saves to config - - Test without local path doesn't save to config - - Test tilde expansion in paths - - Test nested directory creation - -**Manual testing completed:** -- All 10 project commands tested end-to-end -- Path normalization verified -- Database sync trigger verified -- Cleanup on remove verified -- Bisync state reset verified - -## Future Enhancements (Out of Scope) - -- **Per-project rclone profiles**: Allow advanced users to override defaults -- **Conflict resolution UI**: Interactive conflict resolution for bisync -- **Sync scheduling**: Automatic periodic sync without watch mode -- **Sync analytics**: Track sync frequency, data transferred, etc. -- **Multi-machine coordination**: Detect and warn about concurrent edits from different machines diff --git a/specs/SPEC-3 Agent Definitions.md b/specs/SPEC-3 Agent Definitions.md deleted file mode 100644 index 9bd52970..00000000 --- a/specs/SPEC-3 Agent Definitions.md +++ /dev/null @@ -1,108 +0,0 @@ ---- -title: 'SPEC-3: Agent Definitions' -type: spec -permalink: specs/spec-3-agent-definitions -tags: -- agents -- roles -- process ---- - -# SPEC-3: Agent Definitions - -This document defines the specialist agents used in our specification-driven development process. - -## system-architect - -**Role**: High-level system design and architectural decisions - -**Responsibilities**: -- Create architectural specifications and ADRs -- Analyze system-wide impacts and trade-offs -- Design component interfaces and data flow -- Evaluate technical approaches and patterns -- Document architectural decisions and rationale - -**Expertise Areas**: -- System architecture and design patterns -- Technology evaluation and selection -- Scalability and performance considerations -- Integration patterns and API design -- Technical debt and refactoring strategies - -**Typical Specs**: -- System architecture overviews -- Component decomposition strategies -- Data flow and state management -- Integration and deployment patterns - -## vue-developer - -**Role**: Frontend component development and UI implementation - -**Responsibilities**: -- Create Vue.js component specifications -- Implement responsive UI components -- Design component APIs and interfaces -- Optimize for performance and accessibility -- Document component usage and patterns - -**Expertise Areas**: -- Vue.js 3 Composition API -- Nuxt 3 framework patterns -- shadcn-vue component library -- Responsive design and CSS -- TypeScript integration -- State management with Pinia - -**Typical Specs**: -- Individual component specifications -- UI pattern libraries -- Responsive design approaches -- Component interaction flows - -## python-developer - -**Role**: Backend development and API implementation - -**Responsibilities**: -- Create backend service specifications -- Implement APIs and data processing -- Design database schemas and queries -- Optimize performance and reliability -- Document service interfaces and behavior - -**Expertise Areas**: -- FastAPI and Python web frameworks -- Database design and operations -- API design and documentation -- Authentication and security -- Performance optimization -- Testing and validation - -**Typical Specs**: -- API endpoint specifications -- Database schema designs -- Service integration patterns -- Performance optimization strategies - -## Agent Collaboration Patterns - -### Handoff Protocol -1. Agent receives spec through `/spec implement [name]` -2. Agent reviews spec and creates implementation plan -3. Agent documents progress and decisions in spec -4. Agent hands off to another agent if cross-domain work needed -5. Final agent updates spec with completion status - -### Communication Standards -- All agents update specs through basic-memory MCP tools -- Document decisions and trade-offs in spec notes -- Link related specs and components -- Preserve context for future reference - -### Quality Standards -- Follow existing codebase patterns and conventions -- Write tests that validate spec requirements -- Document implementation choices -- Consider maintainability and extensibility diff --git a/specs/SPEC-4 Notes Web UI Component Architecture.md b/specs/SPEC-4 Notes Web UI Component Architecture.md deleted file mode 100644 index 8191f1fd..00000000 --- a/specs/SPEC-4 Notes Web UI Component Architecture.md +++ /dev/null @@ -1,311 +0,0 @@ ---- -title: 'SPEC-4: Notes Web UI Component Architecture' -type: note -permalink: specs/spec-4-notes-web-ui-component-architecture -tags: -- frontend -- 'component-architecture' -- vue -- 'refactoring' ---- - -# SPEC-4: Notes Web UI Component Architecture - -## Why - -The current Notes.vue component is a monolithic component that handles multiple responsibilities, making it difficult to maintain, test, and understand. This leads to: - -- Complex state management across multiple concerns -- Difficult to isolate and test individual features -- Hard to understand the full scope of functionality -- Circular refactoring cycles when making changes -- Poor separation of concerns between navigation, display, and interaction logic - -We need to decompose this into focused, single-responsibility components that are easier to develop, test, and maintain while preserving the existing functionality users expect. - -## What - -This spec defines the component architecture for decomposing the Notes web UI into focused components with clear responsibilities and interactions. - -**Affected Areas:** -- `/apps/web/components/notes/Notes.vue` - Will be decomposed into smaller components -- `/apps/web/components/notes/` - New component structure -- Existing composables: `useNotesNavigation`, `useNotesFiltering`, `useNotesLayout` -- Mobile responsive behavior and layout management - -**Component Breakdown:** - -``` -┌───────────────────────┬─────────────────────────────────────┬────────────────────────────────────────────────────────────┐ -│ [Project] │ [Project Name] A/Z | ^ │ [edit | view] [actions] │ -├───────────────────────┼─────────────────────────────────────┤ │ -│ All Notes ├─────────────────────────────────────┼────────────────────────────────────────────────────────────┤ -│ Recent │ search... │ [note header] │ -│ [Project base dir] ├─────────────────────────────────────┤ │ -│ ├─────────────────────────────────────┤ │ -│ Folder1 │ Title [modified] │ │ -│ Folder2 │ ├────────────────────────────────────────────────────────────┤ -│ - Nested │ snippet │ [note body] │ -│ │ │ │ -│ │ │ │ -│ ├─────────────────────────────────────┤ │ -│ ├─────────────────────────────────────┤ │ -│ │ │ │ -│ │ │ │ -│ │ │ │ -│ │ │ │ -│ │ │ │ -│ ├─────────────────────────────────────┤ │ -│ ├─────────────────────────────────────┤ │ -│ │ │ │ -│ │ │ │ -│ │ │ │ -│ │ │ │ -│ │ │ │ -│ ├─────────────────────────────────────┤ │ -│ │ │ │ -│ │ │ │ -│ │ │ │ -│ │ │ │ -└───────────────────────┴─────────────────────────────────────┴────────────────────────────────────────────────────────────┘ -``` - - -### ProjectSwitcher Component -- **Location**: Top-left dropdown -- **Responsibility**: Allow users to switch between Basic Memory projects -- **Behavior**: Selecting different project controls entire Notes page content -- **State**: When switching projects, reset to "All notes" view - -### NotesNav Component -- **Views**: Three mutually exclusive options: - - **All notes**: Display all notes in project alphabetically - - **Recent**: Display all notes in project by updated time (desc) - - **Project**: Display notes in top-level directory of project -- **Interaction**: Only one view can be active at a time -- **Folder Integration**: All/Recent ignore folder selection; Project respects folder selection - -### FolderTree Component -- **Display**: Nested list of all folders in project as tree view -- **Interaction**: Selecting folder filters notes in NotesList using directoryList API -- **Navigation Integration**: Selecting folder automatically switches NotesNav to "Project" view for clear UX -- **API Integration**: Uses directoryList API call via useDirectoryListQuery for folder-specific note fetching -- **State Coordination**: Folder selection coordinates with navigation state for intuitive user experience - -### NotesList Component -- **Display**: Vertically scrolling cards showing note summaries -- **Information per card**: - - Note title - - Modified time (relative, e.g., "7 minutes ago") - - Short summary of note content (one line preview) -- **Behavior**: Updates based on NotesNav selection and FolderTree filtering - -### NoteDetail Component -- **Display**: Full content of selected note -- **Sections**: - - Header: Displays frontmatter information - - Content: Note body content -- **Editing**: Current textarea implementation (rich editor in future spec) -- **Frontmatter**: Leave current implementation (enhancement in future spec) - -## How (High Level) - -### Component Architecture Approach -1. **Single Responsibility**: Each component handles one primary concern -2. **Clear Data Flow**: Props down, events up pattern for component communication -3. **Composable Integration**: Use existing composables for state management -4. **Progressive Decomposition**: Extract components incrementally to maintain functionality - -### Implementation Strategy -1. **Extract ProjectSwitcher**: Move project switching logic to dedicated component -2. **Extract NotesNav**: Isolate navigation state and view selection logic -3. **Extract FolderTree**: Separate folder display and selection logic -4. **Extract NotesList**: Isolate note listing and card display logic -5. **Extract NoteDetail**: Separate note content display and editing -6. **Update Notes.vue**: Become orchestration component managing component interactions - -### State Management Integration -- **useNotesNavigation**: Manages navigation state (All/Recent/Project) -- **useNotesFiltering**: Handles filtering logic based on navigation and folder selection -- **useNotesLayout**: Manages responsive layout and panel visibility -- **Component State**: Each component manages its own internal UI state -- **Shared State**: Project selection and note filtering coordinated through composables - -### Responsive Behavior - -Mobile: -- Hide sidebar. pop out panel when selected -- show note list on small screens (existing behavior) -- when note list item is clicked, display note detail on full page. Cancel or go back to return to list - -Desktop: -- Full three-column layout with all components visible - -- **Transitions**: Smooth navigation between mobile panels - -## How to Evaluate - -### Success Criteria -- **Functional Parity**: All existing Notes page functionality preserved -- **Component Isolation**: Each component can be developed/tested independently -- **Clear Responsibilities**: No overlapping concerns between components -- **State Clarity**: Clean data flow and state management patterns -- **Mobile Compatibility**: Responsive behavior maintains current UX -- **Performance**: No degradation in rendering or interaction performance - -### Testing Procedure -1. **Functionality Validation**: - - Project switching works correctly - - All three navigation views (All/Recent/Project) function properly - - Folder selection affects note display appropriately - - Note selection and detail display works - - Mobile responsive behavior preserved - -2. **Component Isolation Testing**: - - Each component can be imported and used independently - - Component props and events are clearly defined - - No tight coupling between components - -3. **Integration Testing**: - - Components communicate correctly through props/events - - State management composables integrate properly - - User workflows function end-to-end - -4. **Performance Validation**: - - Page load time unchanged or improved - - Interaction responsiveness maintained - - Memory usage stable or improved - -### Implementation Validation -- **Code Review**: Clean component structure with single responsibilities -- **Type Safety**: Full TypeScript coverage with proper component prop types -- **Documentation**: Each component has clear interface documentation -- **Tests**: Unit tests for individual components and integration tests for workflows - -## Observations - -- [problem] Monolithic Notes.vue component creates maintenance and testing challenges #component-architecture -- [solution] Component decomposition improves separation of concerns and testability #refactoring -- [pattern] Progressive extraction maintains functionality while improving structure #incremental-improvement -- [interaction] NotesNav and FolderTree have conditional interaction based on selected view #state-management -- [constraint] Mobile responsive behavior must be preserved during decomposition #responsive-design -- [scope] Current editing and frontmatter capabilities remain unchanged #scope-limitation -- [validation] Functional parity is critical success criteria for this refactoring #validation-strategy -- [implementation] Folder selection now properly integrates with directoryList API for accurate filtering #api-integration -- [fix] FolderTree selection functionality completed - works across all navigation views #feature-complete -- [ux-improvement] FolderTree selection automatically switches NotesNav to Project view for clear user feedback #user-experience - -## Relations - -- depends_on [[SPEC-1: Specification-Driven Development Process]] -- implements [[Current Notes.vue functionality]] -- prepares_for [[Future rich editor spec]] -- prepares_for [[Future frontmatter editing spec]] -## Implementation Progress - -### Components - -1. **ProjectSwitcher** (`~/components/notes/ProjectSwitcher.vue`) - - ✅ Top-left dropdown for project switching - - ✅ Integrates with Pinia project store - - ✅ Handles project switching with proper state reset - - ✅ Responsive collapsed/expanded states - - ✅ Expanded menu shows available projects and a Manage Projects option that navigates to the /settings/projects page - - ✅ Simplified component following SortingToggle pattern - clean Props/Emits interface, uses ProjectItem type directly - -2. **NotesNav** (`~/components/notes/NotesNav.vue`) - - ✅ Three mutually exclusive views: All/Recent/Project - - ✅ Dynamic project title based on selected project - - ✅ Clean props down, events up pattern - - ✅ Responsive collapsed/expanded states with tooltips - - ✅ The label for the Project selection should be the folder name for the project, not the project name - -3. **FolderTree** (`~/components/notes/FolderTree.vue`) - - ✅ Nested folder tree view for filtering - - ✅ Uses `useFolderTree()` composable for data - - ✅ Emits `folder-selected` events properly - - ✅ Handles loading, error, and empty states - - ✅ Includes companion `FolderTreeNode.vue` component - - ✅ The current folder should be visibly selected in the tree - -4. **NotesList** (`~/components/notes/NotesList.vue`) - - ✅ Vertically scrolling note summary cards - - ✅ Shows title, updated time (relative), and content preview - - ✅ Badge system for tags with variant logic - - ✅ v-model integration for selectedNote - - ✅ Smooth transitions and animations - - ✅ Contextual title: The current folder name should be displayed at the top of the Notes list, or "All Notes", or "Recent" if they are selected - - ✅ The title header should contain a toggle component to allow sorting with Lucide icon labels - - sorting options: - - name (asc/desc) - default - - file updated time (asc/desc) - - If "Recent" notes nav option is selected the default order should be updated in descending order (recent first) - -5. **NoteDisplay** (`~/components/notes/NoteDisplay.vue` - equivalent to spec's NoteDetail) - - ✅ Full note content display - - ✅ Edit/view mode toggle - - ✅ Header with frontmatter information - - ✅ Markdown rendering capabilities - - ✅ Current textarea implementation preserved - -### Architecture Requirements - -1. **Component Isolation**: Each component can be developed/tested independently ✅ -2. **Single Responsibility**: Each component handles one primary concern ✅ -3. **Clear Data Flow**: Props down, events up pattern implemented ✅ -4. **Composable Integration**: Uses existing composables for state management ✅ -5. **Responsive Behavior**: Mobile/desktop layout preserved ✅ - -### State Management Integration - -- **useNotesNavigation**: Manages navigation state (All/Recent/Project) ✅ -- **useNotesFiltering**: Handles filtering logic based on navigation and folder selection ✅ -- **useNotesLayout**: Manages responsive layout and panel visibility ✅ -- **Component State**: Each component manages its own internal UI state ✅ - -### Interaction Logic - -- Only one NotesNav view active at a time ✅ -- All/Recent views ignore folder selection ✅ -- Project view respects folder selection ✅ -- Project switching resets to "All notes" view ✅ - -### TypeScript Coverage - -- All components have full TypeScript coverage ✅ -- Component props and events properly typed ✅ -- No TypeScript errors in codebase ✅ - -### Success Criteria Validation - -1. **Functional Parity**: All existing Notes page functionality preserved ✅ -2. **Component Isolation**: Each component can be developed/tested independently ✅ -3. **Clear Responsibilities**: No overlapping concerns between components ✅ -4. **State Clarity**: Clean data flow and state management patterns ✅ -5. **Mobile Compatibility**: Responsive behavior maintains current UX ✅ -6. **Performance**: No degradation in rendering or interaction performance ✅ - -## Implementation Decisions - -### Architectural Patterns - -1. **Composition API + `