feat: add Claude Code skills Memanto memory bridge#613
Open
attaboy11 wants to merge 8 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a pluggable memory backend and a SkillMemoryBridge that injects recalled memories before a skill and persists labeled memories after a skill; includes a runnable cross-skill demo, GIF generator, README, requirements, and unit tests covering recall ranking and recovery. ChangesClaude Code Skills + Memanto Memory Bridge
Sequence DiagramsequenceDiagram
participant Skill
participant SkillMemoryBridge
participant BaseMemoryBackend
participant MemantoCliBackend
participant FileMemoryBackend
Skill->>SkillMemoryBridge: before_skill(SkillRun)
SkillMemoryBridge->>SkillMemoryBridge: _query_for(run)
SkillMemoryBridge->>BaseMemoryBackend: recall(query, limit)
BaseMemoryBackend-->>SkillMemoryBridge: list of memories
SkillMemoryBridge-->>Skill: formatted memory context
Skill->>Skill: execute with context
Skill->>SkillMemoryBridge: after_skill(run, transcript)
SkillMemoryBridge->>SkillMemoryBridge: extract labeled lines via regex
loop for each labeled line
SkillMemoryBridge->>SkillMemoryBridge: _memory_type(label)
SkillMemoryBridge->>BaseMemoryBackend: remember(content, type, tags)
end
BaseMemoryBackend-->>SkillMemoryBridge: persist
SkillMemoryBridge-->>Skill: list of stored memories
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
examples/claudecode-skills-memanto/memory_backends.py (1)
160-173: ⚡ Quick winAdd a timeout to the CLI subprocess call.
_run(and_ensure_agent) invoke a CLI that reaches the Moorcheh backend over the network. Without atimeout, a hung request blocks the demo indefinitely. A small timeout with a clear error keeps the example resilient.♻️ Proposed change
def _run(cmd: list[str], *, capture: bool = False) -> str: try: result = subprocess.run( cmd, capture_output=True, text=True, check=True, + timeout=30, ) except OSError as exc: raise _missing_memanto_error() from exc + except subprocess.TimeoutExpired as exc: + raise RuntimeError("Memanto CLI command timed out") from exc except subprocess.CalledProcessError as exc:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/claudecode-skills-memanto/memory_backends.py` around lines 160 - 173, The _run function (and the related _ensure_agent caller) must pass a timeout to subprocess.run to avoid hanging; add a small sensible timeout (e.g., 10 seconds) as the timeout arg in subprocess.run, and catch subprocess.TimeoutExpired to raise a clear RuntimeError like "Memanto CLI command timed out" (include any available stdout/stderr in the message); update any callers such as _ensure_agent if they call _run without expecting timeouts so errors propagate cleanly.examples/claudecode-skills-memanto/requirements.txt (1)
2-2: 💤 Low valueRemove unused
python-dotenvfrom this example
Nodotenvimports orload_dotenvcalls are present inexamples/claudecode-skills-memanto, sopython-dotenv>=1.0.0inexamples/claudecode-skills-memanto/requirements.txtcan likely be dropped (unless it’s used by code outside this directory).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/claudecode-skills-memanto/requirements.txt` at line 2, The requirements file lists python-dotenv (python-dotenv>=1.0.0) but the example code in examples/claudecode-skills-memanto does not import dotenv or call load_dotenv; remove the unused dependency from examples/claudecode-skills-memanto/requirements.txt by deleting the python-dotenv>=1.0.0 line (or move it to a higher-level shared requirements file if another part of the repository actually needs it), and run a quick install/test of the example to ensure nothing else relies on python-dotenv.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/claudecode-skills-memanto/memory_backends.py`:
- Around line 109-128: The remember method is calling the memanto CLI with
undocumented flags (--source and --tags) which can cause unrecognized-argument
errors; update the _run invocation in remember to only use supported CLI args
(keep "--type" and the content) and remove "--source" and "--tags", and if you
need to record provenance or tags attach them to the content string or use a
documented API/flag instead; locate the remember function and its _run call and
replace the unsupported flags that reference self.agent_id and tags with a
supported approach (e.g., embed provenance/tags in content or use the memanto
client/library method).
- Around line 130-139: The recall() implementation scrapes free-form memanto
stdout and filters banner lines, which is brittle; update recall to invoke
memanto via a stable, structured channel or parse a stable payload section
instead of line-based heuristics: modify the call site that uses
_run(["memanto", "recall", ...]) so it requests or extracts a machine-readable
block (e.g., detect and parse a JSON/YAML block if memanto emits one, or call a
memanto library/API instead of the CLI), then parse that structured payload into
the list[str] returned by recall(); ensure you remove the fragile
startswith("MEMANTO","Agent:") filtering and handle parsing errors with clear
exceptions in recall() so failures aren’t silently ignored.
- Around line 141-153: The current _ensure_agent falls back to running
_run(["memanto", "agent", "activate", self.agent_id]) on any non-zero return
code from subprocess.run, which can mask other failures; change it to inspect
the failed process result (the create variable from subprocess.run inside
_ensure_agent) and only call _run activate when the stderr/stdout indicates the
agent already exists (e.g., contains "already exists" or similar message),
otherwise raise an error (include create.stderr/create.stdout) so other create
failures surface; keep the existing OSError -> _missing_memanto_error behavior
intact.
In `@examples/claudecode-skills-memanto/README.md`:
- Around line 66-84: The snippet calls SkillMemoryBridge.before_skill and
after_skill with keyword args but the bridge API expects a SkillRun object;
create a SkillRun instance (e.g., run = SkillRun(skill_name="/tdd", task="Add
tests for invoice webhook idempotency", file_paths=[...])) and pass that to
bridge.before_skill(run) to get memory_context, build the prompt and call
run_skill, then call bridge.after_skill(run, result.transcript) (do not pass
file_paths to after_skill).
---
Nitpick comments:
In `@examples/claudecode-skills-memanto/memory_backends.py`:
- Around line 160-173: The _run function (and the related _ensure_agent caller)
must pass a timeout to subprocess.run to avoid hanging; add a small sensible
timeout (e.g., 10 seconds) as the timeout arg in subprocess.run, and catch
subprocess.TimeoutExpired to raise a clear RuntimeError like "Memanto CLI
command timed out" (include any available stdout/stderr in the message); update
any callers such as _ensure_agent if they call _run without expecting timeouts
so errors propagate cleanly.
In `@examples/claudecode-skills-memanto/requirements.txt`:
- Line 2: The requirements file lists python-dotenv (python-dotenv>=1.0.0) but
the example code in examples/claudecode-skills-memanto does not import dotenv or
call load_dotenv; remove the unused dependency from
examples/claudecode-skills-memanto/requirements.txt by deleting the
python-dotenv>=1.0.0 line (or move it to a higher-level shared requirements file
if another part of the repository actually needs it), and run a quick
install/test of the example to ensure nothing else relies on python-dotenv.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: db0cfefa-162e-448a-ab88-3acf7cb66e72
⛔ Files ignored due to path filters (1)
examples/claudecode-skills-memanto/assets/demo.gifis excluded by!**/*.gif
📒 Files selected for processing (6)
examples/claudecode-skills-memanto/README.mdexamples/claudecode-skills-memanto/make_demo_gif.pyexamples/claudecode-skills-memanto/memory_backends.pyexamples/claudecode-skills-memanto/requirements.txtexamples/claudecode-skills-memanto/run_cross_skill_demo.pyexamples/claudecode-skills-memanto/skill_memory_bridge.py
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
examples/claudecode-skills-memanto/memory_backends.py (2)
84-88:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate the loaded JSON shape here.
json.loads(...)accepts any valid JSON. If the file contains{}or a list with non-object entries,remember()will fail on.append(...)orrecall()will fail on.get(...)later. Normalize unexpected shapes to the existing warning/fallback path here.💡 Minimal fix
def _load(self) -> list[dict[str, str]]: if not self.path.exists(): return [] try: - return json.loads(self.path.read_text(encoding="utf-8")) + payload = json.loads(self.path.read_text(encoding="utf-8")) + if not isinstance(payload, list) or not all( + isinstance(item, dict) for item in payload + ): + raise json.JSONDecodeError("Expected a list of objects", "", 0) + return payload except json.JSONDecodeError: warnings.warn( f"Ignoring malformed demo memory file: {self.path}", RuntimeWarning, stacklevel=2,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/claudecode-skills-memanto/memory_backends.py` around lines 84 - 88, The _load method currently returns whatever json.loads(self.path.read_text(...)) yields which can be any JSON shape; update _load to validate that the parsed value is strictly a list of dict[str, str] (each item a mapping with string keys and string values), and if not, log a warning and return the empty-list fallback (or coerce safe shapes into that format) so subsequent methods like remember() and recall() (which rely on .append and .get) never receive invalid types; reference the _load function, remember(), and recall() when adding the validation and warning logic and ensure the fallback path is used on any unexpected shape.
61-71:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrite the file store atomically.
This rewrites the entire JSON file in place. If the process is interrupted mid-write, the store can become truncated, and
_load()will then treat the whole history as empty on the next run. Write to a temp file in the same directory andreplace()it instead.💡 Minimal fix
def remember( self, content: str, *, memory_type: str = "learning", tags: str = "claudecode,skills,memanto", ) -> None: self.path.parent.mkdir(parents=True, exist_ok=True) records = self._load() records.append( MemoryRecord( content=content, memory_type=memory_type, source=self.source, tags=tags, ).to_json() ) - self.path.write_text(json.dumps(records, indent=2) + "\n", encoding="utf-8") + payload = json.dumps(records, indent=2) + "\n" + tmp_path = self.path.with_suffix(f"{self.path.suffix}.tmp") + tmp_path.write_text(payload, encoding="utf-8") + tmp_path.replace(self.path)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/claudecode-skills-memanto/memory_backends.py` around lines 61 - 71, The current append flow loads records via self._load(), appends a MemoryRecord JSON and calls self.path.write_text(...) which can truncate the store if interrupted; change the write to perform an atomic write: create a temporary file in the same directory (e.g., using tempfile.NamedTemporaryFile or a Path-based temp name), write json.dumps(records, indent=2) to that temp file with utf-8 encoding, flush and fsync to disk, then atomically replace the original file with Path.replace(temp_path, self.path). Update the code paths that call self.path.write_text(...) to use this temp-write-and-replace approach so _load() never sees a truncated file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@examples/claudecode-skills-memanto/memory_backends.py`:
- Around line 84-88: The _load method currently returns whatever
json.loads(self.path.read_text(...)) yields which can be any JSON shape; update
_load to validate that the parsed value is strictly a list of dict[str, str]
(each item a mapping with string keys and string values), and if not, log a
warning and return the empty-list fallback (or coerce safe shapes into that
format) so subsequent methods like remember() and recall() (which rely on
.append and .get) never receive invalid types; reference the _load function,
remember(), and recall() when adding the validation and warning logic and ensure
the fallback path is used on any unexpected shape.
- Around line 61-71: The current append flow loads records via self._load(),
appends a MemoryRecord JSON and calls self.path.write_text(...) which can
truncate the store if interrupted; change the write to perform an atomic write:
create a temporary file in the same directory (e.g., using
tempfile.NamedTemporaryFile or a Path-based temp name), write
json.dumps(records, indent=2) to that temp file with utf-8 encoding, flush and
fsync to disk, then atomically replace the original file with
Path.replace(temp_path, self.path). Update the code paths that call
self.path.write_text(...) to use this temp-write-and-replace approach so _load()
never sees a truncated file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 647aeb47-9441-4367-b226-1abc46b54709
📒 Files selected for processing (3)
examples/claudecode-skills-memanto/README.mdexamples/claudecode-skills-memanto/memory_backends.pyexamples/claudecode-skills-memanto/requirements.txt
💤 Files with no reviewable changes (1)
- examples/claudecode-skills-memanto/requirements.txt
✅ Files skipped from review due to trivial changes (1)
- examples/claudecode-skills-memanto/README.md
Author
|
Follow-up for the review thread: I addressed the CodeRabbit findings in What changed:
Validation run locally:
|
Author
|
Added the organic social showcase link to the PR description: https://x.com/ciprianimacovei/status/2061044237206950149?s=46&t=lY3sRT0pRNlqtaJrHuIX4w |
Author
|
Follow-up hardening pushed in |
Author
|
Follow-up hardening pushed in What changed:
Validation rerun locally:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
examples/claudecode-skills-memanto/skill_memory_bridge.py (1)
110-115: 💤 Low valueOptional: sanitize
skill_namemore robustly before using it as a tag.
strip("/")only trims leading/trailing slashes. Askill_namecontaining commas or whitespace would corrupt the comma-joined tag string built inafter_skill(Line 99). The currentFileMemoryBackenddoesn't parsetagsduring recall, so this is non-functional today, but a more durable backend could mis-split tags.♻️ Optional hardening
- skill_tag = run.skill_name.strip("/") + skill_tag = run.skill_name.strip("/").replace(",", " ").strip()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/claudecode-skills-memanto/skill_memory_bridge.py` around lines 110 - 115, The _tags_for function currently uses run.skill_name.strip("/") which won't prevent commas or whitespace from corrupting the comma-joined tag string built in after_skill; update _tags_for to robustly sanitize the skill name (e.g., trim outer whitespace/slashes, replace or remove commas and internal whitespace, collapse multiple non-alphanumeric chars to a single safe separator like "-" or "_" and lower-case the result, and drop empty results) before appending to self.base_tags so tags are safe and stable across backends; locate and modify _tags_for (and verify after_skill's tag join still expects comma-separated tags) to return the sanitized tag tuple.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@examples/claudecode-skills-memanto/skill_memory_bridge.py`:
- Around line 110-115: The _tags_for function currently uses
run.skill_name.strip("/") which won't prevent commas or whitespace from
corrupting the comma-joined tag string built in after_skill; update _tags_for to
robustly sanitize the skill name (e.g., trim outer whitespace/slashes, replace
or remove commas and internal whitespace, collapse multiple non-alphanumeric
chars to a single safe separator like "-" or "_" and lower-case the result, and
drop empty results) before appending to self.base_tags so tags are safe and
stable across backends; locate and modify _tags_for (and verify after_skill's
tag join still expects comma-separated tags) to return the sanitized tag tuple.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 92a1c02d-571e-419d-a8c7-f71df985a7b1
📒 Files selected for processing (3)
examples/claudecode-skills-memanto/README.mdexamples/claudecode-skills-memanto/skill_memory_bridge.pyexamples/claudecode-skills-memanto/tests/test_skill_memory_bridge.py
✅ Files skipped from review due to trivial changes (1)
- examples/claudecode-skills-memanto/README.md
Author
|
Follow-up hardening pushed in What changed:
Validation rerun locally:
|
Author
|
Follow-up for CodeRabbit's tag-safety note: pushed What changed:
Validation:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #508.
Summary
examples/claudecode-skills-memanto, a lightweight before/after lifecycle bridge for developer skills.before_skill(...)recalls relevant engineering memory and returns a compact context block for the next skill prompt.after_skill(...)extracts durable decisions, preferences, quirks, constraints, and learnings from a completed skill transcript and stores them through Memanto.examples/claudecode-skills-memanto/assets/demo.gif.Social showcase
Organic showcase link: https://x.com/ciprianimacovei/status/2061044237206950149?s=46&t=lY3sRT0pRNlqtaJrHuIX4w
Latest review state
752f3bfVerification
Demo output includes:
Malformed-memory and atomic-write smoke check also passed with the offline backend safely recovering from an unexpected JSON shape.
Summary by CodeRabbit
New Features
Documentation
Tests