eidetic-memory: refresh wrappers (eidetic 0.10) + memory-discipline convention#2
Open
OriNachum wants to merge 1 commit into
Open
eidetic-memory: refresh wrappers (eidetic 0.10) + memory-discipline convention#2OriNachum wants to merge 1 commit into
OriNachum wants to merge 1 commit into
Conversation
- **Memory-discipline "Conventions and workflow" section in `CLAUDE.md`** — a per-task *recall-before / remember-after* convention (scope localized to this repo's nick) so the vendored `remember` / `recall` skills are actually used, not just present: `/recall` before non-trivial work to build on prior decisions instead of re-deriving them, and `/remember` when a non-obvious decision, constraint, fix-and-why, or hard-won gotcha surfaces. The section documents this repo's memory as **in-repo and public** — records resolve to `<repo-root>/.eidetic/memory` (committed, team- and mesh-shared). Inserted idempotently (skipped if already present), slotted under an existing "Conventions and workflow" heading when one exists, else appended. ### Changed - **Refreshed the `remember` + `recall` wrappers from eidetic-cli 0.10.0** (cite-don't-import) — picks up eidetic's **project-local store default**: the files backend now resolves per record by visibility — PUBLIC records inside a git repo go to `<repo-root>/.eidetic/memory` (committed, team-shared), PRIVATE records (or any record outside a repo) go to `$HOME/.eidetic/memory` (never committed), an explicit `EIDETIC_DATA_DIR` still wins, and recall reads both stores and merges. Also carries the 0.9.3 hardening (interactive-stdin guard, `help` as a search term, SIGPIPE-safe suffix parsing). **Recipe policy override (the wrappers here are NOT byte-verbatim):** the injected default visibility is flipped from eidetic's `private` to **`public`**, so a plain `/remember` lands the note in `./.eidetic/memory` in this repo, kept as part of the repo — pass `--visibility private` to route a record to `$HOME` instead. `remember` drives `eidetic remember` (idempotent upsert of one JSON record or an NDJSON batch on stdin); `recall` drives `eidetic recall` with four search modes (exact / approximate / keyword / hybrid). Each `SKILL.md` is localized only in the illustrative `--scope <nick>` examples (Provenance keeps "First-party to eidetic-cli"). Runtime dep: the `eidetic` CLI on PATH (else a local eidetic-cli checkout with `uv`) — **`eidetic >= 0.10.0`** for the in-repo routing; on an older CLI the public records still work but are stored in `$HOME/.eidetic/memory` instead of in-repo. Propagated by rollout-cli's `eidetic-memory` recipe.
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
|
PR Summary by QodoRefresh eidetic remember/recall wrappers (0.10) and document memory discipline Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
17 rules✅ Skills:
doc-test-alignment, version-bump, cicd 1. recall.sh modified in vendored skill
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+9
to
+13
| # The store is the files backend. Default location resolves per-operation: | ||
| # PUBLIC records inside a git repo → <repo-root>/.eidetic/memory (committed, | ||
| # team-shared); PRIVATE records, or any record outside a git repo → | ||
| # $HOME/.eidetic/memory (never committed). Recall reads both stores and merges. | ||
| # An explicit EIDETIC_DATA_DIR wins and short-circuits to that single dir. |
There was a problem hiding this comment.
1. recall.sh modified in vendored skill 📘 Rule violation ⚙ Maintainability
This PR directly edits .claude/skills/recall/scripts/recall.sh and .claude/skills/remember/scripts/remember.sh, which are vendored skill files that must not be modified in-repo. These changes risk being overwritten by the next guildmaster re-sync and create upstream drift/divergence unless made upstream and re-synced.
Agent Prompt
## Issue description
This PR includes direct edits to vendored skill files under `.claude/skills/` (`.claude/skills/recall/scripts/recall.sh` and `.claude/skills/remember/scripts/remember.sh`), which violates the policy against in-repo modifications of these synced assets.
## Issue Context
Vendored skills should be updated upstream and then re-synced from guildmaster (or by updating the sync configuration / upstream source). Otherwise, local edits will drift/diverge from upstream and may be overwritten on the next guildmaster re-sync.
## Fix Focus Areas
- .claude/skills/recall/scripts/recall.sh[9-13]
- .claude/skills/remember/scripts/remember.sh[15-19]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+76
to
+84
| # No record argument AND stdin is an interactive terminal → `eidetic remember` | ||
| # would block forever waiting for NDJSON. Show usage instead of hanging. A piped | ||
| # or redirected stdin (`cat records.ndjson | remember.sh`) is not a TTY and | ||
| # proceeds to the batch path normally. | ||
| if [ "$#" -eq 0 ] && [ -t 0 ]; then | ||
| usage >&2 | ||
| printf 'hint: pass a JSON record as an argument, or pipe NDJSON on stdin.\n' >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
2. Flags-only remember can hang 🐞 Bug ☼ Reliability
The new interactive-stdin guard only checks for zero arguments, so remember.sh --json (or any flags-only call) still reaches eidetic remember with no record argument and an interactive stdin, causing an indefinite block waiting for NDJSON. This can stall CI/agent runs and defeats the intended hardening.
Agent Prompt
## Issue description
`remember.sh` adds an interactive-stdin guard, but it only triggers when `$# == 0`. If the user passes only flags (e.g. `remember.sh --json`) and stdin is a TTY, `eidetic remember` will still block forever waiting for NDJSON.
## Issue Context
The wrapper’s contract is: either a single JSON record as the first argument, or NDJSON via stdin. When stdin is interactive and there is no JSON record argument, we should fail fast with usage.
## Fix Focus Areas
- .claude/skills/remember/scripts/remember.sh[76-85]
ⓘ 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.
Added
CLAUDE.md— aper-task recall-before / remember-after convention (scope localized to this
repo's nick) so the vendored
remember/recallskills are actually used,not just present:
/recallbefore non-trivial work to build on priordecisions instead of re-deriving them, and
/rememberwhen a non-obviousdecision, constraint, fix-and-why, or hard-won gotcha surfaces. The section
documents this repo's memory as in-repo and public — records resolve to
<repo-root>/.eidetic/memory(committed, team- and mesh-shared). Insertedidempotently (skipped if already present), slotted under an existing
"Conventions and workflow" heading when one exists, else appended.
Changed
remember+recallwrappers from eidetic-cli 0.10.0(cite-don't-import) — picks up eidetic's project-local store default: the
files backend now resolves per record by visibility — PUBLIC records inside a
git repo go to
<repo-root>/.eidetic/memory(committed, team-shared), PRIVATErecords (or any record outside a repo) go to
$HOME/.eidetic/memory(nevercommitted), an explicit
EIDETIC_DATA_DIRstill wins, and recall reads bothstores and merges. Also carries the 0.9.3 hardening (interactive-stdin guard,
helpas a search term, SIGPIPE-safe suffix parsing). Recipe policyoverride (the wrappers here are NOT byte-verbatim): the injected default
visibility is flipped from eidetic's
privatetopublic, so a plain/rememberlands the note in./.eidetic/memoryin this repo, kept as partof the repo — pass
--visibility privateto route a record to$HOMEinstead.
rememberdriveseidetic remember(idempotent upsert of one JSONrecord or an NDJSON batch on stdin);
recalldriveseidetic recallwithfour search modes (exact / approximate / keyword / hybrid). Each
SKILL.mdislocalized only in the illustrative
--scope <nick>examples (Provenance keeps"First-party to eidetic-cli"). Runtime dep: the
eideticCLI on PATH (else alocal eidetic-cli checkout with
uv) —eidetic >= 0.10.0for thein-repo routing; on an older CLI the public records still work but are stored
in
$HOME/.eidetic/memoryinstead of in-repo. Propagated by rollout-cli'seidetic-memoryrecipe.