-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix/mcp reasoningbank integration #818
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
the PR is compatable with v2.7.0-alpha.14 - verified the issue still exists in v2.7.0-alpha.14 and this fixes it |
## Merge Summary
Merged upstream/main (v2.7.1) into fix/mcp-reasoningbank-integration branch.
## Conflicts Resolved
### 1. Metrics Files
- Removed .claude-flow/metrics/*.json (should be gitignored)
### 2. MCP Server - neural_train API Mismatch
**Problem**: Upstream v2.7.1 added neural_train code for `this.memoryStore`
but this branch refactored to `this.memoryManager`
**Solution**: Applied same fix as local-main:
- Changed `this.memoryStore` → `this.memoryManager` (7 instances)
- Fixed API calls:
- `.store(key, value, {namespace, ttl, metadata})` → `.store(key, value, namespace, metadata)`
- `.retrieve(key, {namespace})` → `.get(key, namespace)?.value`
- `.list({namespace})` → `.query(pattern, {namespace, limit})`
## Changes from Upstream
- ✅ v2.7.1 release (MCP pattern persistence)
- ✅ AgentDB skills expansion
- ✅ Commands migrated to skills system
- ✅ Comprehensive testing suite
- ✅ Docker verification infrastructure
## Neural Train Fix
Adapted upstream's v2.7.1 neural_train feature to work with UnifiedMemoryManager:
```javascript
// ✅ NOW WORKING:
if (this.memoryManager) {
await this.memoryManager.store(modelId, patternData, 'patterns', {...});
const stats = await this.memoryManager.get(statsKey, 'pattern-stats');
}
```
## Files Changed
- src/mcp/mcp-server.js (112 insertions, 112 deletions)
- dist-cjs/src/mcp/mcp-server.js (rebuilt)
- Removed: Old .claude/commands/** (migrated to skills)
- Added: v2.7.1 test suite and documentation
## Testing
✅ Syntax validation passed
✅ Build successful
✅ Ready for PR ruvnet#818 update
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
🔄 Updated for v2.7.1 CompatibilityThis PR has been updated to resolve conflicts with upstream's v2.7.1 release. Changes in This Update1. Merged upstream/main (v2.7.1)
2. Fixed neural_train API Mismatch Upstream v2.7.1 added Applied fixes: // ❌ Upstream v2.7.1 (doesn't work with this PR's refactoring):
if (this.memoryStore) {
await this.memoryStore.retrieve(key, {namespace: 'patterns'});
}
// ✅ Fixed for UnifiedMemoryManager:
if (this.memoryManager) {
const entry = await this.memoryManager.get(key, 'patterns');
const value = entry?.value;
}API transformations applied:
Files Updated
Testing✅ Build successful ResultThis PR now provides:
Ready for review! 🎉 |
Resolves issue #7 by integrating MCP memory tools with UnifiedMemoryManager and ReasoningBank backend, enabling semantic search for MCP users. - Added ReasoningBank detection and auto-initialization - Implemented priority-based storage: ReasoningBank → SQLite → JSON - Added semantic search routing for query operations - Updated get(), getStats(), delete(), clearNamespace() for ReasoningBank - Added storeReasoningBank() and queryReasoningBank() methods - Enhanced getStorageInfo() to report ReasoningBank capabilities - Replaced fallback-store with UnifiedMemoryManager - Updated handleMemoryUsage() to use new UnifiedMemoryManager API - Updated handleMemorySearch() for semantic search support - Fixed swarm storage operations to use unified memory - Added storage_type and semantic_search flags to responses - Comprehensive solution documentation - Test results and verification - CLI-MCP interoperability confirmation ✅ Semantic search for MCP users when ReasoningBank initialized ✅ CLI-MCP data interoperability (shared ReasoningBank backend) ✅ Auto-detection with graceful JSON fallback ✅ Storage type reporting in MCP responses ✅ 100% backward compatible - ✅ ReasoningBank detection works correctly - ✅ CLI stores data accessible via MCP - ✅ MCP stores data accessible via CLI - ✅ Semantic search returns similarity scores - ✅ Stats show correct storage type 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
659a7ef to
5649bf2
Compare
✅ PR Cleaned Up - Rebased on upstream/mainUpdated approach: Rebased instead of merging to keep PR diff clean. What ChangedBefore (bad approach):
After (correct approach):
Commit Structure After RebaseThe Additional FixAdded one commit to adapt upstream's v2.7.1 Changes in that commit:
ResultThis PR now cleanly shows:
No redundant v2.7.1 files - those are already in the base branch! 🎯 |
## Issue
Upstream v2.7.1 added neural_train code using memoryStore API:
```javascript
await this.memoryStore.store(key, value, {namespace, ttl, metadata})
const value = await this.memoryStore.retrieve(key, {namespace})
const list = await this.memoryStore.list({namespace, limit})
```
This PR refactored to UnifiedMemoryManager with different API:
```javascript
await this.memoryManager.store(key, value, namespace, metadata)
const entry = await this.memoryManager.get(key, namespace)
const list = await this.memoryManager.query(pattern, {namespace, limit})
```
## Solution: Backward Compatibility Layer
Instead of changing upstream's neural_train code, add compatibility to UnifiedMemoryManager.
### Changes to UnifiedMemoryManager
**1. Enhanced store() to accept both APIs:**
```javascript
// OLD API: store(key, value, {namespace, ttl, metadata})
// NEW API: store(key, value, namespace, metadata)
async store(key, value, namespaceOrOptions = 'default', metadata = {}) {
// Auto-detect which API is being used
if (typeof namespaceOrOptions === 'object') {
// Old API - extract from options object
namespace = namespaceOrOptions.namespace || 'default';
metadata = namespaceOrOptions.metadata || {};
} else {
// New API - use parameters directly
namespace = namespaceOrOptions;
}
// ... proceed with storage
}
```
**2. Added retrieve() method (wraps get()):**
```javascript
async retrieve(key, options = {}) {
const namespace = options.namespace || 'default';
const entry = await this.get(key, namespace);
return entry?.value || null; // Returns just the value
}
```
**3. Added list() method (wraps query()):**
```javascript
async list(options = {}) {
const namespace = options.namespace || 'default';
const limit = options.limit || 100;
return await this.query('', { namespace, limit });
}
```
### Changes to MCP Server
**Added memoryStore alias:**
```javascript
this.memoryManager = getUnifiedMemory();
this.memoryStore = this.memoryManager; // Backward compatibility
```
## Benefits
✅ No changes needed to upstream's neural_train code
✅ Both old and new APIs work simultaneously
✅ Clean upgrade path for future code
✅ Minimal PR footprint
## Testing
✅ Old API: store({namespace, ttl, metadata}) - WORKS
✅ Old API: retrieve({namespace}) - WORKS
✅ Old API: list({namespace, limit}) - WORKS
✅ New API: store(namespace, metadata) - WORKS (still works)
✅ Alias: this.memoryStore === this.memoryManager - WORKS
## Files Changed
- src/memory/unified-memory-manager.js (3 new methods)
- src/mcp/mcp-server.js (1 line: alias)
- dist-cjs/* (rebuilt)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
5649bf2 to
77efb10
Compare
✅ Better Solution: Backward Compatibility LayerChanged approach to be much cleaner - instead of modifying upstream's neural_train code, I added backward compatibility to UnifiedMemoryManager. Why This Is Better❌ Previous approach:
✅ New approach:
Implementation1. Enhanced // ✅ OLD API (what v2.7.1 uses):
await manager.store(key, value, {namespace, ttl, metadata})
// ✅ NEW API (what this PR uses):
await manager.store(key, value, namespace, metadata)
// Auto-detects which API based on 3rd parameter type\!2. Added async retrieve(key, {namespace}) {
const entry = await this.get(key, namespace);
return entry?.value || null;
}3. Added async list({namespace, limit}) {
return await this.query('', {namespace, limit});
}4. Added MCP server alias: this.memoryManager = getUnifiedMemory();
this.memoryStore = this.memoryManager; // Backward compatibilityResult✅ Upstream's v2.7.1 neural_train code works unmodified Testing Verified// ✅ Old API works
await this.memoryStore.store(key, value, {namespace, metadata});
const value = await this.memoryStore.retrieve(key, {namespace});
const list = await this.memoryStore.list({namespace, limit});
// ✅ New API works
await this.memoryManager.store(key, value, namespace, metadata);
const entry = await this.memoryManager.get(key, namespace);
const results = await this.memoryManager.query(pattern, {namespace, limit});This is a much more elegant solution! 🎯 |
🧹 Cleaned Up PR - Removed Build ArtifactsRemoved all What's Left in PRSource files only:
No more:
Benefits✅ Cleaner PR diff The PR is now focused on source code changes only! 🎯 |
…ryStore API Brings in backward compatibility layer from PR ruvnet#818: - UnifiedMemoryManager now supports both old and new APIs - memoryStore alias added to MCP server - v2.7.1 neural_train code works without modification - All builds and tests passing Conflicts resolved: - Kept imports from local-main (configCommand, etc.) - Added memoryStore alias from fix branch - Accepted backward-compat methods in UnifiedMemoryManager
…ryStore API Brings in backward compatibility layer from PR ruvnet#818: - UnifiedMemoryManager now supports both old and new APIs - memoryStore alias added to MCP server - v2.7.1 neural_train code works without modification - All builds and tests passing Conflicts resolved: - Kept imports from local-main (configCommand, etc.) - Added memoryStore alias from fix branch - Accepted backward-compat methods in UnifiedMemoryManager
🔗 Unify MCP and CLI Memory Storage with ReasoningBank Integration
Fixes #812
Summary
MCP memory tools (
memory_usage,memory_search) and CLI memory commands previously used separate storagesystems, preventing MCP users from accessing ReasoningBank's semantic search capabilities. This PR unifies
both interfaces to share the same storage backend with automatic ReasoningBank detection.
Problem
Before this fix:
.swarm/memory.db) with semantic searchmemory_searchMCP tool returned success but no actual resultsExample of broken behavior: