admin/basicmachines-co-basic-memory
1
0
Fork 0
mirror of https://github.com/basicmachines-co/basic-memory synced 2026-06-21 13:47:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: SPEC-20 enhancements - cleanup, path normalization, and docs

Browse Source 
This commit adds several critical improvements discovered during
manual testing of SPEC-20 project-scoped rclone sync:

**Critical Bug Fixes:**

1. Path Normalization (fixes path doubling bug)
   - API: Strip /app/data/ prefix in project_router.py
   - CLI: Defensive normalization in project.py
   - Rclone: Fix get_project_remote() path construction
   - Prevents files syncing to /app/data/app/data/project/

2. Rclone Flag Fix
   - Changed --filters-file to correct --filter-from flag
   - Fixes "unknown flag" error in sync and bisync

**Enhancements:**

3. Automatic Database Sync
   - POST to /{project}/project/sync after file operations
   - Keeps database in sync with files automatically
   - Skipped on --dry-run operations

4. Enhanced Project Removal
   - Clean up local sync directory (with --delete-notes)
   - Always remove bisync state directory
   - Always remove cloud_projects config entry
   - Informative messages about what was/wasn't deleted

5. Bisync State Reset Command
   - New: bm project bisync-reset <project>
   - Clears corrupted bisync metadata
   - Safe recovery tool for bisync issues

6. Improved Project List UI
   - Show Local Path column in cloud mode
   - Conditionally show/hide columns based on config
   - Prevent path truncation with no_wrap/overflow
   - Apply path normalization to display

**Documentation:**

7. Cloud CLI Documentation
   - Add troubleshooting: empty directory bisync issues
   - Add troubleshooting: bisync state corruption
   - Document bisync-reset command usage
   - Explain rclone bisync limitations

8. SPEC-20 Updates
   - Mark implementation complete
   - Document all enhancements in Implementation Notes
   - Update phase checklists with completed work
   - Add manual testing results

**Tests:**

9. Unit Tests for --local-path
   - Test config persistence with --local-path
   - Test no config without --local-path
   - Test tilde expansion
   - Test nested directory creation

All changes tested manually end-to-end.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: phernandez <paul@basicmachines.co>
This commit is contained in:
phernandez
2025-10-28 17:36:36 -05:00
parent db85186e37
commit d749c7737f
7 changed files with 630 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files
specs/SPEC-20 Simplified Project-Scoped Rclone Sync.md
+156 -20
View File
@@ -1,7 +1,8 @@
---
title: 'SPEC-20: Simplified Project-Scoped Rclone Sync'
date: 2025-01-27
status: Draft
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
@@ -1019,11 +1020,15 @@ rm -rf ~/basic-memory-cloud-sync/
- [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
- [ ] Update `project list` to show sync status (optional)
- [ ] Update `cloud/core_commands.py`: Simplify `cloud setup` command (optional)
- [ ] Add helper functions: `get_all_sync_projects()`, `get_project_by_name()` (optional)
- [ ] Write integration tests for new commands (deferred)
- [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)
@@ -1053,23 +1058,154 @@ rm -rf ~/basic-memory-cloud-sync/
- [x] Update tests to remove references to deprecated functionality
- [x] All typecheck errors resolved
### Phase 6: Documentation
- [ ] Update `docs/cloud-cli.md` with new workflow
- [ ] Add migration guide for existing users
- [ ] Update command reference
- [ ] Add troubleshooting section
- [ ] Update SPEC-8 with "Superseded by SPEC-20" note
- [ ] Add examples for common workflows
### 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
- [ ] Test Scenario 1: New user setup
- [ ] Test Scenario 2: Multiple projects
- [ ] Test Scenario 3: Project without sync
- [ ] Test Scenario 4: Integrity check
- [ ] Test Scenario 5: Safety features (max delete)
- [ ] Verify performance targets (setup < 30s, sync < 5s)
- [ ] Test migration from SPEC-8 implementation
### 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 <project>`
**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)