fix: resolve issue #508#623
Conversation
📝 WalkthroughWalkthroughThis PR adds a complete example implementation that integrates Memanto persistent developer memory with Claude Code skill execution. It introduces a lifecycle hook ( ChangesMemanto Skills Integration Example
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
🤖 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_hook.py`:
- Around line 87-91: The current code appends only the basename (p.name) to
query_parts which collapses different paths like src/index.css and
docs/index.css into the same memory scope; change it to use the full path (e.g.,
str(p) or p.as_posix()) when appending the file identifier and keep p.suffix for
the extension if desired, updating both occurrences that manipulate file_path /
p / query_parts so memories are scoped by full file path instead of just the
basename.
- Around line 109-123: post_skill_execute() and pre_skill_execute() are
currently promoting raw model output into persistent "strict" constraints;
change the logic so that only memories with an explicit user-authored or
validated source are persisted as authoritative and all LLM-derived memories are
saved with a source flag (e.g., "llm") and treated as advisory. In
post_skill_execute(), attach a source/validated flag to each memory and only
persist when source == "user" or validated==true; in pre_skill_execute(), build
the prompt from memories by including the source and by changing the wording
(remove "Please align your actions strictly with these constraints") to mark
LLM-derived memories as "advisory/context" while keeping user memories as
actionable. Also ensure any future persistence path checks the source/validated
metadata before treating content as mandatory.
In `@examples/claudecode-skills-memanto/README.md`:
- Around line 49-53: Update the README instructions to show correct .env file
syntax by removing the shell "export" usage and demonstrating the plain
KEY=VALUE form for MOORCHEH_API_KEY; also clarify the alternative (instead of
"or copy your Moorcheh API key") by stating the other option explicitly (for
example, paste the key into the .env file or set the MOORCHEH_API_KEY
environment variable in your shell) and reference the environment variable name
MOORCHEH_API_KEY so users know exactly what to name the entry.
In `@examples/claudecode-skills-memanto/skills_simulator.py`:
- Around line 102-111: The code reads MOORCHEH_API_KEY using Path.home() but
Path is not imported and the env-file parsing uses split("=") which can error;
import Path from pathlib at the top of skills_simulator.py, then in the fallback
parsing block (the logic that sets api_key) replace line.split("=")[1].strip()
with robust parsing using line.partition("=")[2].strip() and only assign api_key
if the partitioned value is non-empty to avoid NameError and improve key
extraction.
🪄 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: 06860623-382d-4523-a10e-81929a6da097
📒 Files selected for processing (6)
.gitignoreexamples/claudecode-skills-memanto/README.mdexamples/claudecode-skills-memanto/memory_hook.pyexamples/claudecode-skills-memanto/requirements.txtexamples/claudecode-skills-memanto/skills_simulator.pyexamples/claudecode-skills-memanto/test_skills.py
| if file_path: | ||
| p = Path(file_path) | ||
| query_parts.append(f"file: {p.name}") | ||
| if p.suffix: | ||
| query_parts.append(f"extension: {p.suffix}") |
There was a problem hiding this comment.
Use the full file path for memory scoping, not just the basename.
Right now src/index.css and docs/index.css are both reduced to index.css, so recall/store can cross-contaminate unrelated memories. That weakens the “current file path/task” contract from the issue.
💡 Suggested fix
if file_path:
p = Path(file_path)
- query_parts.append(f"file: {p.name}")
+ normalized_path = p.as_posix()
+ query_parts.append(f"path: {normalized_path}")
+ query_parts.append(f"file: {p.name}")
if p.suffix:
query_parts.append(f"extension: {p.suffix}")
@@
if file_path:
- tags.append(Path(file_path).name)
+ tags.append(f"path:{Path(file_path).as_posix()}")Also applies to: 184-185
🤖 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_hook.py` around lines 87 - 91, The
current code appends only the basename (p.name) to query_parts which collapses
different paths like src/index.css and docs/index.css into the same memory
scope; change it to use the full path (e.g., str(p) or p.as_posix()) when
appending the file identifier and keep p.suffix for the extension if desired,
updating both occurrences that manipulate file_path / p / query_parts so
memories are scoped by full file path instead of just the basename.
| prompt_lines = [ | ||
| "\n==================================================", | ||
| "💡 [Memanto Persistent Developer Memory Context]", | ||
| "The following relevant engineering choices, preferences,", | ||
| "and codebase quirks were recalled for this context:", | ||
| ] | ||
|
|
||
| for i, mem in enumerate(memories, 1): | ||
| mem_type = mem.get("type", "fact") | ||
| title = mem.get("title", "Untitled") | ||
| content = mem.get("content", "") | ||
| prompt_lines.append(f" {i}. [{mem_type.upper()}] {title}: {content}") | ||
|
|
||
| prompt_lines.append("Please align your actions strictly with these constraints.") | ||
| prompt_lines.append("==================================================\n") |
There was a problem hiding this comment.
Don’t promote untrusted model output into future hard constraints.
post_skill_execute() persists snippets taken directly from output_text, and pre_skill_execute() later injects every recalled memory as something the model should follow “strictly”. That lets a bad or injected completion become a persistent system-level instruction for later skills.
At minimum, only persist explicitly user-authored preferences as authoritative, and render LLM-derived memories as advisory context unless they’ve been validated/confirmed.
Also applies to: 163-205
🤖 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_hook.py` around lines 109 - 123,
post_skill_execute() and pre_skill_execute() are currently promoting raw model
output into persistent "strict" constraints; change the logic so that only
memories with an explicit user-authored or validated source are persisted as
authoritative and all LLM-derived memories are saved with a source flag (e.g.,
"llm") and treated as advisory. In post_skill_execute(), attach a
source/validated flag to each memory and only persist when source == "user" or
validated==true; in pre_skill_execute(), build the prompt from memories by
including the source and by changing the wording (remove "Please align your
actions strictly with these constraints") to mark LLM-derived memories as
"advisory/context" while keeping user memories as actionable. Also ensure any
future persistence path checks the source/validated metadata before treating
content as mandatory.
| Create/edit a `.env` file in the repository root or copy your Moorcheh API key: | ||
|
|
||
| ```bash | ||
| export MOORCHEH_API_KEY="your-api-key-here" | ||
| ``` |
There was a problem hiding this comment.
Fix .env file syntax and clarify instructions.
The example shows shell export syntax, but .env files don't use the export keyword. This could confuse users or cause the environment variable to not load correctly with dotenv parsers.
Additionally, line 49's phrasing "or copy your Moorcheh API key" is unclear about what the alternative to creating a .env file would be.
📝 Proposed fix
-Create/edit a `.env` file in the repository root or copy your Moorcheh API key:
+Create a `.env` file in the repository root with your Moorcheh API key:
```bash
-export MOORCHEH_API_KEY="your-api-key-here"
+MOORCHEH_API_KEY=your-api-key-here</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
Create a `.env` file in the repository root with your Moorcheh API key:
🤖 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/README.md` around lines 49 - 53, Update
the README instructions to show correct .env file syntax by removing the shell
"export" usage and demonstrating the plain KEY=VALUE form for MOORCHEH_API_KEY;
also clarify the alternative (instead of "or copy your Moorcheh API key") by
stating the other option explicitly (for example, paste the key into the .env
file or set the MOORCHEH_API_KEY environment variable in your shell) and
reference the environment variable name MOORCHEH_API_KEY so users know exactly
what to name the entry.
| # Read Moorcheh API Key | ||
| api_key = os.environ.get("MOORCHEH_API_KEY") | ||
| if not api_key: | ||
| # Check standard config file fallback | ||
| _env_path = Path.home() / ".memanto" / ".env" | ||
| if _env_path.exists(): | ||
| with open(_env_path) as f: | ||
| for line in f: | ||
| if line.startswith("MOORCHEH_API_KEY="): | ||
| api_key = line.split("=")[1].strip() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=$(fd -p 'skills_simulator.py' | head -n1)
sed -n '1,25p' "$file"
printf '\n--- Path usage ---\n'
rg -n '\bPath\b' "$file"Repository: moorcheh-ai/memanto
Length of output: 786
Import Path before using the fallback env-file path
examples/claudecode-skills-memanto/skills_simulator.py (lines 106-111) uses Path.home() but Path isn’t imported, causing a NameError when MOORCHEH_API_KEY is missing. Harden the key parsing by using partition("=") instead of split("=")[1].
💡 Suggested fix
import os
import sys
import time
+from pathlib import Path
@@
if _env_path.exists():
with open(_env_path) as f:
for line in f:
if line.startswith("MOORCHEH_API_KEY="):
- api_key = line.split("=")[1].strip()
+ _, _, api_key = line.partition("=")
+ api_key = api_key.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/skills_simulator.py` around lines 102 -
111, The code reads MOORCHEH_API_KEY using Path.home() but Path is not imported
and the env-file parsing uses split("=") which can error; import Path from
pathlib at the top of skills_simulator.py, then in the fallback parsing block
(the logic that sets api_key) replace line.split("=")[1].strip() with robust
parsing using line.partition("=")[2].strip() and only assign api_key if the
partitioned value is non-empty to avoid NameError and improve key extraction.
|
/attempt #508 Submitted PR: #717 Social showcase: https://x.com/TerminusProto/status/2064451356392661020?s=20 Reviewer-safe demo repo: https://github.com/cwwjacobs/mdemo |
|
We were blown away by the community's creativity and the sheer volume of high-quality submissions! After reviewing all the pull requests against the bounty's success matrix, we have decided to move forward with merging PR #692, which implemented a highly portable prompt-injection architecture via CLAUDE.md. We are closing this PR because it falls into one of the architectural approaches that we ultimately decided against for the ecosystem:
We deeply appreciate the time and engineering effort you put into this submission. The codebase was fantastic to review, and we hope to see you in future Moorcheh bounties! |
3 participants
Resolves #508.
Implements fix for "[BOUNTY $100] 🐜 The Memanto + mattpocock Developer Skills Challenge". All tests and validation run successfully.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
.gitignoreto exclude pipeline output files.