Add eidetic remember/recall memory skills - #6
Merged
Conversation
- **Vendored the `remember` + `recall` memory skills from eidetic-cli** (cite-don't-import) — the write/read halves of eidetic's shared `~/.eidetic/memory` surface, so this agent (Claude and its colleague backend) can persist facts across sessions and recall them later, sharing one store. `remember` drives `eidetic remember` (idempotent upsert of one JSON record or an NDJSON batch on stdin, dedup by id + content hash); `recall` drives `eidetic recall` with four search modes — exact / approximate / keyword / hybrid — each hit carrying text, full provenance metadata, a relevance score, and a freshness signal. The `.sh` wrappers are byte-verbatim from eidetic-cli (their first-party origin); each `SKILL.md` is localized only in the illustrative `--scope <nick>` examples (Provenance keeps "First-party to eidetic-cli"). Both default to this agent's PRIVATE scope, reading the suffix from `culture.yaml`. Runtime dep: the `eidetic` CLI on PATH (else a local eidetic-cli checkout with `uv`). Propagated by rollout-cli's `eidetic-memory` recipe.
|
PR Summary by QodoVendor eidetic remember/recall memory skills into .claude/skills Description
Diagram
High-Level Assessment
Files changed (6)
|
Code Review by Qodo
1. recall.sh lacks dry-run
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| : "${EIDETIC_EMBED_MODEL:=Qwen/Qwen3-Embedding-0.6B}" | ||
| export EIDETIC_EMBED_URL EIDETIC_EMBED_MODEL | ||
|
|
||
| exec "${EIDETIC[@]}" recall "${SCOPE_ARGS[@]}" "$@" |
There was a problem hiding this comment.
1. recall.sh lacks dry-run 📘 Rule violation ☼ Reliability
The new recall.sh and remember.sh wrappers unconditionally execute eidetic recall/`eidetic remember without a default dry-run mode or an explicit --apply gate, even though recall` is described as performing passive reinforcement writes. This can lead to unintended persistence or state changes during routine invocations that may appear read-oriented or low-risk.
Agent Prompt
## Issue description
Both `recall.sh` and `remember.sh` currently run `eidetic recall` / `eidetic remember` unconditionally while forwarding user flags/args, and they do not enforce a safe default (dry-run) or require an explicit `--apply` opt-in for write side effects. Update these wrappers to comply with Rule 731643 by making write-capable behavior require explicit user intent (e.g., defaulting to dry-run and only applying/persisting when `--apply` is provided), especially since `recall` is documented to perform passive reinforcement writes.
## Issue Context
`/recall` and `/remember` are skill entry points intended to be invoked routinely; unsafe defaults increase the chance of unintended persistence, state changes, and test pollution. The recall skill documentation explicitly notes that recall mutates record fields (such as `last_recall`/`recall_count`), so invoking it without an apply gate can cause unexpected writes even when the command seems read-oriented.
## Fix Focus Areas
- .claude/skills/recall/scripts/recall.sh[46-65]
- .claude/skills/recall/scripts/recall.sh[141-141]
- .claude/skills/recall/SKILL.md[9-10]
- .claude/skills/recall/SKILL.md[95-97]
- .claude/skills/remember/scripts/remember.sh[52-66]
- .claude/skills/remember/scripts/remember.sh[125-138]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+18
to
+44
| resolve_eidetic() { | ||
| if command -v eidetic >/dev/null 2>&1; then | ||
| EIDETIC=(eidetic) # installed console script — the normal case | ||
| return 0 | ||
| fi | ||
| # Dev fallback: inside the eidetic-cli checkout, run via uv. | ||
| local dir | ||
| dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) | ||
| while [ -n "$dir" ] && [ "$dir" != "/" ]; do | ||
| if [ -f "$dir/pyproject.toml" ] \ | ||
| && grep -q '^name = "eidetic-cli"' "$dir/pyproject.toml" 2>/dev/null; then | ||
| if command -v uv >/dev/null 2>&1; then | ||
| EIDETIC=(uv run --project "$dir" eidetic) | ||
| return 0 | ||
| fi | ||
| break | ||
| fi | ||
| dir=$(dirname "$dir") | ||
| done | ||
| cat >&2 <<'EOF' | ||
| error: eidetic CLI not found. | ||
| hint: install it with `uv tool install eidetic-cli` (or `pipx install eidetic-cli`), | ||
| or run from inside the eidetic-cli checkout with `uv` available. | ||
| The console script is `eidetic` (dist name: eidetic-cli). | ||
| EOF | ||
| return 1 | ||
| } |
There was a problem hiding this comment.
2. Skill scripts require external cli 📘 Rule violation ⚙ Maintainability
The new skill scripts depend on an external eidetic binary on PATH (or an external eidetic-cli checkout invoked via uv), rather than using only in-repo, vendored dependencies. This breaks the requirement that skill scripts not depend on resources outside the repository.
Agent Prompt
## Issue description
The skill entry-point scripts dynamically resolve and execute an external tool (`eidetic`) from `PATH` or via an external checkout (`uv run --project ...`). Compliance requires skill scripts to avoid dependencies that live outside this repository.
## Issue Context
Depending on `PATH`-resolved binaries is both non-reproducible and vulnerable to environment differences or path hijacking.
## Fix Focus Areas
- .claude/skills/recall/scripts/recall.sh[16-44]
- .claude/skills/remember/scripts/remember.sh[23-50]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+12
to
+14
| --include-shadowed / --include-archived to retrieve them. The store lives at | ||
| ~/.eidetic/memory (a home-dir path outside any git worktree); the wrapper | ||
| defaults queries to this agent's PERSONAL, PRIVATE scope (`--scope lecodeur |
There was a problem hiding this comment.
3. Home-dir path committed 📘 Rule violation § Compliance
The PR introduces the per-machine home-directory path ~/.eidetic/memory in tracked documentation and scripts. Per compliance, machine-specific paths must be confined to git-ignored .claude/skills.local.yaml rather than committed files.
Agent Prompt
## Issue description
Tracked skill docs/scripts embed a per-machine filesystem location (`~/.eidetic/memory`). Compliance requires that per-machine paths live only in git-ignored `.claude/skills.local.yaml` (or equivalent local-only config), not in committed files.
## Issue Context
Committing home-dir paths reduces portability and can mislead users about correct configuration in different environments.
## Fix Focus Areas
- .claude/skills/recall/SKILL.md[12-14]
- .claude/skills/remember/SKILL.md[11-14]
- .claude/skills/recall/scripts/recall.sh[9-12]
- .claude/skills/remember/scripts/remember.sh[15-19]
- CHANGELOG.md[12-25]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+94
to
+132
| resolve_scope() { | ||
| local dir suffix="" | ||
| dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) | ||
| while [ -n "$dir" ] && [ "$dir" != "/" ]; do | ||
| if [ -f "$dir/culture.yaml" ]; then | ||
| # Capture only the first non-space token after `suffix:` (so an | ||
| # inline `# comment` or trailing space can't bleed into the scope), | ||
| # then strip surrounding quotes only — matching the canonical parser | ||
| # in .claude/skills/cicd/scripts/_resolve-nick.sh. | ||
| suffix=$(sed -n \ | ||
| 's/^[[:space:]]*-\{0,1\}[[:space:]]*suffix:[[:space:]]*\([^[:space:]]*\).*/\1/p' \ | ||
| "$dir/culture.yaml" | head -n1 | tr -d "\"'") | ||
| break | ||
| fi | ||
| dir=$(dirname "$dir") | ||
| done | ||
| printf '%s' "$suffix" | ||
| } | ||
|
|
||
| has_flag() { | ||
| local needle=$1 | ||
| shift | ||
| local a | ||
| for a in "$@"; do | ||
| case "$a" in | ||
| "$needle" | "$needle"=*) return 0 ;; | ||
| esac | ||
| done | ||
| return 1 | ||
| } | ||
|
|
||
| SCOPE_ARGS=() | ||
| if ! has_flag --scope "$@"; then | ||
| EIDETIC_SCOPE=$(resolve_scope) | ||
| if [ -n "$EIDETIC_SCOPE" ]; then | ||
| SCOPE_ARGS+=(--scope "$EIDETIC_SCOPE") | ||
| has_flag --visibility "$@" || SCOPE_ARGS+=(--visibility private) | ||
| fi | ||
| fi |
There was a problem hiding this comment.
4. Silent scope fallback 🐞 Bug ⛨ Security
In remember.sh/recall.sh, --scope <suffix> and default --visibility private are only injected when resolve_scope extracts a non-empty suffix; if culture.yaml exists but doesn’t match the expected single-line suffix: <token> shape, the wrapper silently falls back to eidetic CLI defaults (wrong scope/visibility). This can cause writes/reads to land outside the intended personal private scope, risking incorrect recall behavior and unintended sharing semantics.
Agent Prompt
## Issue description
`remember.sh` and `recall.sh` silently skip scope injection when `culture.yaml` exists but `suffix` cannot be parsed (empty result). That bypasses the intended default of querying/writing in the agent’s personal scope with private visibility.
## Issue Context
The repo already has a canonical suffix parser (`.claude/skills/cicd/scripts/_resolve-nick.sh`) that warns when parsing fails; the new skills should behave similarly (warn or error) rather than silently falling back.
## Fix Focus Areas
- .claude/skills/remember/scripts/remember.sh[94-132]
- .claude/skills/recall/scripts/recall.sh[94-132]
## Suggested fix
- If a `culture.yaml` is found but the parsed suffix is empty, emit a clear stderr warning and **exit non-zero** (fail closed), or at minimum warn loudly.
- Consider anchoring the search to the git repo root (like `_resolve-nick.sh`) and/or using the same parsing approach (python regex) to reduce drift.
- Optionally validate the parsed suffix (non-empty, does not start with `-`) before using it as the `--scope` value.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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.
Vendors eidetic-cli's first-party
remember+recallmemory skills into this repo's.claude/skills/kit (cite-don't-import), giving this agent a shared, persistent memory surface (~/.eidetic/memory) that Claude and the colleague backend both read and write.remember→eidetic remember: idempotent upsert of one JSON record or an NDJSON batch on stdin (dedup by id + content hash).recall→eidetic recall: four search modes (exact / approximate / keyword / hybrid), each hit carrying text, full provenance metadata, a relevance score, and a freshness signal.The
.shwrappers are byte-verbatim from eidetic-cli (their first-party origin); this repo'sSKILL.mdscope examples are localized to its own nick. Version bumped + CHANGELOG updated per the AgentCulture rule. Runtime dep: theeideticCLI on PATH (else a local eidetic-cli checkout +uv).Propagated by rollout-cli's
eidetic-memoryrecipe (origin: agentculture/eidetic-cli). Squash-merge at your discretion.🤖 Generated with Claude Code