mirror of
https://github.com/rohitg00/agentmemory.git
synced 2026-09-14 20:16:33 +08:00
Rohit Ghumare
f72c7a54fc
* fix: Claude Code plugin auto-wires MCP + sandbox-safe skills (#139) Two fixes reported in #139: 1. The Claude Code plugin (rohitg00/agentmemory) only shipped hooks and skills; it did not auto-register any MCP server. Users had to configure the @agentmemory/mcp stdio server separately, which the README didn't make clear. Add plugin/.mcp.json so /plugin install agentmemory@agentmemory wires up the MCP server automatically when the plugin is enabled. 2. The `recall` and `session-history` skills used inline bash pre-execution blocks (`!$(curl -s ... ${VAR:-default})`) with shell expansion, which Claude Code's sandbox rejects by pattern match with "Contains expansion" — no way to bypass except for Claude to reconstruct the curl call manually, as the reporter had to work around. Rewrite all four plugin skills (recall / remember / forget / session-history) as pure prompts that tell Claude to use the MCP tools directly: memory_smart_search, memory_save, memory_sessions, memory_governance_delete. No bash, no sandbox issues, no shell escaping. Skills also run faster — no curl subprocess fork on every invocation. Also tighten the README Claude Code install snippet to note that the plugin now registers hooks + skills AND auto-wires the MCP server in one step. Bumps to 0.8.9. 707 tests still pass (no behavioral changes to server code; only plugin assets). * review: implement the MCP tools the new skills call (#139) CodeRabbit flagged that the rewritten plugin skills reference memory_smart_search and memory_governance_delete, but the standalone MCP shim (@agentmemory/mcp) only exposed 5 tools: memory_save, memory_recall, memory_sessions, memory_export, memory_audit. At runtime the skills would have thrown "Unknown tool" the first time they ran. Add the missing handlers to src/mcp/standalone.ts: - memory_smart_search — aliases memory_recall, falling back to substring search over the in-memory KV store. The standalone shim has no engine behind it, so BM25/vector/graph aren't available, but the substring match is good enough for plugin usage on small personal memory stores. - memory_governance_delete — deletes memories by id. Accepts memoryIds as an array or a comma-separated string, returns {deleted, requested, reason}. Throws on missing/empty input, silently skips unknown ids. Also normalize memory_save concepts/files: they now accept both arrays (what the new skills send) and CSV strings (what the old skills and legacy clients send). New normalizeList() helper centralizes the logic. Fix memory_sessions to honour a limit arg (default 20) — it was previously returning every session in the store. Update the forget skill wording per CodeRabbit's nit: make it explicit that memory_governance_delete only takes memoryIds, and that deleting a whole session means collecting all memory IDs from that session via search first. Adds 8 regression tests to test/mcp-standalone.test.ts covering: - memory_save with array concepts/files - memory_save with CSV concepts/files (legacy path) - memory_smart_search substring fallback - memory_sessions limit - memory_governance_delete array path - memory_governance_delete CSV path - memory_governance_delete missing/empty validation - memory_governance_delete unknown-id skip Full suite: 715 passing (was 707 + 8 new). * review: reject empty search query + fix skill troubleshooting paths (#139) CodeRabbit round 2: 1. src/mcp/standalone.ts memory_smart_search accepted '' as a query and used .includes('') which matches every memory. In the forget flow this would surface unrelated memory IDs for deletion. Now throws 'query is required' on missing/empty/ whitespace-only input. Also broadens the search corpus from just title+content to include files, concepts, sessionIds, and id — so users can /forget by file path, concept name, or session ID, as the skill docs already promise. 2. All four skill files (recall, remember, forget, session-history) said 'check the agentmemory engine is running on localhost:3111' when tools are missing. Wrong troubleshooting path: the stdio MCP shim spawned by Claude Code via .mcp.json has no REST API, no port, no localhost. The correct checks are /plugin list, /mcp, and restarting Claude Code after install. Updated all four skills. Adds 2 new regression tests to test/mcp-standalone.test.ts: - memory_smart_search rejects empty/whitespace/missing query - memory_smart_search finds memories by file, concept, or id Full suite: 717 passing. * review: tighten content/limit validation in standalone MCP (#139) CodeRabbit round 3: 1. memory_save cast args.content as string and called .trim(), which throws a runtime TypeError if content is a number, object, null, etc. Replace the cast with a proper typeof guard that returns the clean 'content is required' error for any non-string input. 2. memory_smart_search trimmed rawQuery for emptiness but then lowercased the *untrimmed* value, so a query like ' bcrypt ' would fail substring matches with trailing whitespace. Trim first, then lowercase. 3. (args.limit as number) || 10 is fragile — it treats 0 and NaN the same, silently accepts negative/Infinity values, and has no upper bound so a malicious or buggy caller could request a million memories in one slice(). Add parseLimit() helper that: - rejects non-number/non-string types (booleans, objects, arrays) - rejects NaN, Infinity, and non-positive values - clamps to MAX_LIMIT=100 via Math.min - Math.floor()s to force integer - falls back to a per-call default (10 for search, 20 for sessions, 50 for audit) Applied to all three limit sites: memory_smart_search, memory_sessions, memory_audit. Adds 2 regression tests: - memory_save rejects 42 / {} / [] / null / undefined / true without crashing on .trim() - parseLimit falls back on bogus values and clamps 99999 -> 100 Full suite: 719 passing (was 717 + 2 new).