feat: add basic-memory watch CLI command#559
Conversation
Standalone long-running file watcher that reuses SyncCoordinator without starting the MCP server. Useful for running Basic Memory sync alongside external tools like openclaw-basic-memory. - Add `quiet` field to SyncCoordinator, threaded through to WatchService - Add `quiet` param to `initialize_file_sync()` (defaults True for MCP) - New `watch.py` command: init → sync coordinator → block until signal → clean shutdown - Supports `--project` flag to constrain to a single project Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
|
Claude finished @phernandez's task in 2m 10s —— View job PR Review: feat: add basic-memory watch CLI commandSummaryThis PR successfully implements a standalone Code Quality & Standards ✅Strengths
Architecture Alignment
Testing & Coverage ✅Test Quality
Coverage
Parameter Threading ✅The
This design correctly allows:
Specific File Reviewssrc/basic_memory/cli/commands/watch.py:20-87Excellent implementation with clear narrative flow:
Minor observations:
tests/cli/test_watch.py:1-228Thorough test coverage with good patterns:
One consideration:
src/basic_memory/sync/coordinator.py:58Clean parameter addition:
src/basic_memory/services/initialization.py:72-107Parameter added correctly:
tests/sync/test_coordinator.py:26-148Three focused tests validating the
Clean, focused tests that verify the threading works correctly. Security & Performance ✅
Compatibility ✅Windows Compatibility
Console Output
Documentation ✅
Integration Points ✅
Final Checklist ResultsCode Quality & Standards
Testing & Documentation
Basic Memory Architecture
Security & Performance
Compatibility
RecommendationsRequired for MergeNone - this PR is ready to merge. Optional Enhancements (Future Work)
ConclusionAPPROVED ✅ This PR demonstrates excellent software engineering:
The implementation correctly separates concerns: the watch command provides Rich console output for interactive CLI use, while the MCP server remains quiet. The Great work on this feature! The code quality and testing are exemplary. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44570a0736
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| loop = asyncio.get_running_loop() | ||
| for sig in (signal.SIGINT, signal.SIGTERM): | ||
| loop.add_signal_handler(sig, _signal_handler) |
There was a problem hiding this comment.
run_watch registers SIGINT/SIGTERM with loop.add_signal_handler(...) unconditionally, but that API is not implemented on Windows event loops, so basic-memory watch fails with NotImplementedError before the watcher starts. This impacts all win32 users of the new command even though the command explicitly tries to support Windows via selector policy setup.
Useful? React with 👍 / 👎.
| project_name, _ = config_manager.get_project(project) | ||
| if not project_name: | ||
| typer.echo(f"No project found named: {project}", err=True) | ||
| raise typer.Exit(1) |
There was a problem hiding this comment.
When --project is invalid, run_watch raises typer.Exit(1) after initialize_app(config) has already run, but before entering the try/finally block that calls db.shutdown_db(). That leaves initialization-opened DB resources unclosed on this error path and can cause noisy or hanging event-loop teardown for users who mistype the project name.
Useful? React with 👍 / 👎.
- Resolve merge conflicts in app.py (add both "reindex" and "watch" to skip_init_commands) and rclone_commands.py (use --header for Tigris consistency) - Address Codex P1: handle Windows where add_signal_handler is unsupported by falling back to signal.signal() - Address Codex P2: ensure DB cleanup on all exit paths (including invalid --project) by wrapping post-init code in try/finally - Add test for Windows signal handler fallback - Apply ruff formatting to files from main Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
|
Claude finished @phernandez's task in 2m 22s —— View job PR Review: feat: add basic-memory watch CLI commandSummaryThis PR successfully implements a standalone Codex Review AnalysisThe codex-connector flagged two issues, but both are false positives:
Code Quality & Standards ✅Strengths
Architecture Alignment
Testing & Coverage ✅Test Quality (8 new tests total)test_watch.py (5 tests):
test_coordinator.py (3 tests):
Coverage ✅
Parameter Threading ✅The
Design correctness:
Basic Memory Checklist ResultsCode Quality & Standards
Testing & Documentation
Basic Memory Architecture
Security & Performance
Compatibility
Notable ImprovementsThis PR includes a 6th test not mentioned in the PR description:
This test ensures the Windows signal handler fallback works correctly, demonstrating thorough attention to cross-platform compatibility. Security Review ✅
Performance Review ✅
Recommendations✅ Ready to MergeThis PR is approved and ready to merge. No changes required. Optional Future WorkThe PR description mentions manual testing still needed:
These are good to verify before merging, but the automated test coverage is comprehensive enough to have high confidence. ConclusionAPPROVED ✅ This is exemplary software engineering work:
The implementation correctly separates concerns: the watch command provides Rich console output for interactive CLI use, while the MCP server remains quiet. The Outstanding work! 🎉 |
1 participant
Summary
basic-memory watchcommand that runs the file watcher as a standalone long-running process without starting the MCP serverquietparameter throughSyncCoordinator→initialize_file_sync→WatchServiceso the watch command shows Rich console output while MCP keeps quiet--projectflag to constrain watching to a single projectTest plan
tests/cli/test_watch.py(init, coordinator args, project env var, invalid project exit, shutdown cleanup)tests/sync/test_coordinator.py(quiet defaults, quiet override, quiet passthrough)basic-memory watchstarts and shows file changes, stops on Ctrl+Cbasic-memory watch --project my-projectconstrains to single project🤖 Generated with Claude Code