From 006d34d23a16dbca12d0dc3859ab8579be9b4c06 Mon Sep 17 00:00:00 2001 From: Ori Nachum Date: Mon, 6 Jul 2026 21:15:57 +0300 Subject: [PATCH 1/4] =?UTF-8?q?contract:=20memory=20scope+visibility=20con?= =?UTF-8?q?vention=20v1=20=E2=80=94=20wrapper=20default=20fixed=20to=20pub?= =?UTF-8?q?lic,=20drift-tested=20(#28)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds docs/contract.md: one convention every eidetic memory-store consumer pins to — a personal scope per repo named by culture.yaml's `suffix` (record families ride the existing `type` field + metadata, no new scope primitives), and a `public` default visibility for in-repo team-shared records (private stays one explicit `--visibility private` away). Includes the store-resolution table copied verbatim from eidetic/memory/backend.py's _resolve_write_dir/_candidate_read_dirs. Fixes the live bug this convention exists to close: the vendored recall/remember skill wrappers here (.claude/skills/{recall,remember}/ scripts/*.sh) silently overrode the plain `eidetic remember`/`recall` CLI's own `--visibility` default (verified: argparse default is already "public" in eidetic/cli/_commands/{remember,recall}.py — no CLI code changed) to "private" for a no-flag invocation, while colleague's separate colleague/memory.py runtime hardcodes "public" for its own shell-outs. Two consumers of the same store disagreeing on the default by accident. Both wrappers now default to public, matching the CLI and colleague; the no-suffix-resolved warning (which only made sense when a silent private->public downgrade was the risk) is removed since there's no longer a privacy-downgrade risk either way. SKILL.md descriptions for both skills updated to match. This is a deliberate reversal of the private-by-default decision recorded in docs/specs/2026-06-23-eidetic-cli-s-remember-recall-skill-wrappers-are-h.md (FIX-4) — recorded honestly in docs/contract.md's History section rather than silently overwritten, since that spec affirmed private was intentional at the time and only fixed a doc/behavior mismatch around it. Provenance note: recall/remember are FIRST-PARTY to eidetic-cli (origin, not vendored from guildmaster — see docs/skill-sources.md), and are fanned out to ~57 downstream repos including colleague's own vendored copy. This fix needs to be propagated DOWNSTREAM via the next re-sync/ rollout to those consumers — it is not itself a change to push upstream. Tests: tests/test_wrapper_convention.py shells both the vendored wrapper and the raw CLI (via python -m eidetic, no uv/network dependency) with EIDETIC_DATA_DIR pointed at a tmp dir and no --visibility flag, asserting both land the same visibility (fails before this fix, passes after) plus a mutual-visibility recall proof. tests/test_contract_drift.py parses contract.md's machine-readable block and cross-checks it against both wrapper scripts' literal default and the CLI's actual argparse default (exercised via a real parse_args() call, not introspection) — a future divergence on any of the three surfaces fails here. Verified: eidetic's own --visibility argparse default was already "public" for both remember and recall (no CLI code touched, per task scope). Full suite: 355 passed, 2 pre-existing skips (unrelated). black/isort/flake8/ bandit/markdownlint-cli2/teken doctor --strict all clean on changed files. Co-Authored-By: Claude Fable 5 --- .claude/skills/recall/SKILL.md | 33 ++-- .claude/skills/recall/scripts/recall.sh | 48 +++-- .claude/skills/remember/SKILL.md | 39 ++-- .claude/skills/remember/scripts/remember.sh | 49 ++--- docs/contract.md | 127 +++++++++++++ tests/test_contract_drift.py | 113 +++++++++++ tests/test_wrapper_convention.py | 199 ++++++++++++++++++++ 7 files changed, 538 insertions(+), 70 deletions(-) create mode 100644 docs/contract.md create mode 100644 tests/test_contract_drift.py create mode 100644 tests/test_wrapper_convention.py diff --git a/.claude/skills/recall/SKILL.md b/.claude/skills/recall/SKILL.md index 3b459cf..8f0ffb7 100644 --- a/.claude/skills/recall/SKILL.md +++ b/.claude/skills/recall/SKILL.md @@ -15,12 +15,13 @@ description: > record outside a git repo, go to $HOME/.eidetic/memory (never committed). An explicit EIDETIC_DATA_DIR wins and short-circuits to that single dir. Recall reads both stores and merges. The wrapper defaults queries to this agent's - PERSONAL, PRIVATE scope (`--scope eidetic-cli --visibility private`, suffix - read from culture.yaml) — matching where /remember writes — so a no-flag - recall returns this agent's own private records plus the shared public pool, - and Claude and the colleague backend recall each other's memories because both - resolve the same suffix via this skill. Use - when the user says "recall", "what do we know about X", "search memory", + PERSONAL, PUBLIC scope (`--scope eidetic-cli --visibility public`, suffix + read from culture.yaml) — the memory scope+visibility convention (v1, + docs/contract.md), matching where /remember writes — so a no-flag recall + returns the full public pool, and Claude and the colleague backend recall + each other's memories because both resolve the same suffix via this skill + (pass --visibility private to also see this agent's own private records). + Use when the user says "recall", "what do we know about X", "search memory", "have we seen X before", "look it up in memory", "eidetic recall", or before answering from scratch when prior context may already be stored. Pairs with the sibling /remember skill. @@ -123,13 +124,15 @@ compete on score/signal just like active ones when included. - `--case-sensitive` — for `--mode exact`. - `--filter KEY=VALUE` — metadata facet filter (repeatable): e.g. `--filter source=docs`. - `--scope NAME` / `--visibility public|private` — scope isolation (no private - leak). **The wrapper defaults this to the agent's PERSONAL, PRIVATE scope** - (`--scope eidetic-cli --visibility private`, suffix read from `culture.yaml`), - matching where `/remember` writes — so a no-flag recall returns this agent's - own private records **plus** the shared public pool, while those private records - stay invisible to a `default`/other-scope recall. Pass `--scope`/`--visibility` - to query elsewhere; a wheel install with no `culture.yaml` falls back to the - CLI default `default`/`public`. + leak). **The wrapper defaults this to the agent's PERSONAL, PUBLIC scope** + (`--scope eidetic-cli --visibility public`, suffix read from `culture.yaml`), + matching where `/remember` writes and the memory scope+visibility convention + (v1, `docs/contract.md`) — so a no-flag recall returns the full public pool. + Pass `--visibility private` to instead query this agent's own private records + **plus** the shared public pool (a private query still sees every public + record; only a private record is scope-isolated). Pass `--scope` to query a + different scope entirely; a wheel install with no `culture.yaml` falls back to + the CLI default `default`/`public`. - `--backend files|mongo|neo4j` — default `files` (the shared home-dir store). - `--include-shadowed` — include shadowed records in results (excluded by default). - `--include-archived` — include archived records in results (excluded by default). @@ -177,6 +180,10 @@ bash .claude/skills/recall/scripts/recall.sh "power" --include-archived --includ Claude + colleague). It is not isolated between OS users by anything but file permissions; keep genuinely private data in a `--visibility private` scope and treat the host as the trust boundary. +- **Scope + visibility convention:** see [`docs/contract.md`](../../../docs/contract.md) + (memory scope+visibility convention, v1) for the naming/default rules this + wrapper (and every other `eidetic remember`/`recall` consumer, including + the colleague backend's `colleague/memory.py`) pins to. ## Provenance diff --git a/.claude/skills/recall/scripts/recall.sh b/.claude/skills/recall/scripts/recall.sh index b164ca1..1f4ef7e 100755 --- a/.claude/skills/recall/scripts/recall.sh +++ b/.claude/skills/recall/scripts/recall.sh @@ -79,7 +79,7 @@ esac resolve_eidetic || exit 2 -# ── default to this agent's PERSONAL, PRIVATE scope (culture.yaml `suffix`) ── +# ── default to this agent's PERSONAL, PUBLIC scope (culture.yaml `suffix`) ─── # Query this agent's OWN personal scope by default, matching where /remember # writes, instead of the global `default` scope shared by every project on this # host. We read the `suffix` from the nearest culture.yaml (walking up from this @@ -88,15 +88,26 @@ resolve_eidetic || exit 2 # backend (running in a worktree of this same repo) resolves the same suffix, # keeping the Claude↔colleague shared-memory story intact. # -# The personal scope is PRIVATE by default to match /remember: in eidetic's model -# a private record is served only to a recall in the SAME scope (`can_serve`), so -# querying with --scope --visibility private is what retrieves those -# isolated records (a public/default recall can't see them). Scope and visibility -# are paired — the private default applies only when we inject the resolved scope, -# and only if the caller didn't pass --visibility (so an explicit -# `--visibility public` still wins). An explicit --scope on the command line takes -# over steering entirely; a wheel install with no culture.yaml falls back to the -# plain CLI default (`default`/`public`). +# The personal scope is PUBLIC by default to match /remember — the memory +# scope+visibility convention (v1, docs/contract.md): a public record is +# visible to ANY query scope regardless of name (`can_serve`, +# eidetic/memory/scope.py), so a no-flag recall here returns the full public +# pool, matching both the plain `eidetic recall` CLI's own --visibility +# default and the colleague backend's runtime (colleague/memory.py hardcodes +# --visibility public). Scope and visibility are paired — the public default +# applies only when we inject the resolved scope, and only if the caller +# didn't pass --visibility (so an explicit `--visibility private` still +# wins). Passing --visibility private restores exactly what used to be the +# implicit default: this agent's own private notes (scope=, matched +# exactly — `can_serve` only serves a private record to a query in the SAME +# scope) PLUS the full public pool (a private query still sees every public +# record too — that "private + public" merge is `can_serve`'s behavior +# whenever the QUERY itself is private, unrelated to this default). What +# changed is only the no-flag case: a plain recall now returns public-only, +# since a public-visibility query structurally excludes every private +# record regardless of scope name. An explicit --scope on the command line +# takes over steering entirely; a wheel install with no culture.yaml falls +# back to the plain CLI default (`default`/`public`). resolve_scope() { local dir suffix="" dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) @@ -136,16 +147,15 @@ 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) - elif ! has_flag --visibility "$@"; then - # No suffix AND no explicit --visibility: the query runs against - # eidetic's own default (scope=default, visibility=public), not this - # agent's private personal scope — so an empty result isn't silently - # misread. Warn on stderr (stdout stays clean for --json). Warn ONLY - # here: an explicit --scope (outer guard) or --visibility (this guard) is - # a deliberate choice, honored verbatim, so either flag silences this. - printf 'warning: no culture.yaml suffix resolved; querying the public default scope rather than a private personal scope. Pass --scope or --visibility to target deliberately.\n' >&2 + has_flag --visibility "$@" || SCOPE_ARGS+=(--visibility public) fi + # No suffix resolved (e.g. a wheel install with no culture.yaml): leave + # --scope/--visibility unset entirely, so the plain `eidetic recall` + # defaults apply (scope=default, visibility=public) — identical + # visibility to the suffix-resolved case above, just grouped under the + # `default` scope name instead of this agent's personal one. Nothing to + # warn about: unlike the old private-by-default behavior, there is no + # unexpected-leak or unexpected-empty-result surprise here either way. fi # Default the embedding endpoint to the local model-gear embed gear. eidetic diff --git a/.claude/skills/remember/SKILL.md b/.claude/skills/remember/SKILL.md index 6e18ffd..6660abb 100644 --- a/.claude/skills/remember/SKILL.md +++ b/.claude/skills/remember/SKILL.md @@ -13,11 +13,13 @@ description: > (committed, team-shared); PRIVATE records, or any record outside a git repo, go to $HOME/.eidetic/memory (never committed). An explicit EIDETIC_DATA_DIR wins and short-circuits to that single dir. The wrapper defaults records to - this agent's PERSONAL, PRIVATE scope (`--scope eidetic-cli --visibility - private`, suffix read from culture.yaml) so they don't leak to a - default/other-scope recall — Claude and the colleague backend still share them - because both resolve the same suffix via this skill. Pass `--visibility public` - to contribute to the shared public pool instead. Use when the user says + this agent's PERSONAL, PUBLIC scope (`--scope eidetic-cli --visibility + public`, suffix read from culture.yaml) — the memory scope+visibility + convention (v1, docs/contract.md) — matching both the plain `eidetic + remember` CLI's own default and the colleague backend's runtime, so a + no-flag remember here and a no-flag `eidetic remember` elsewhere land + mutually-visible records. Pass `--visibility private` to keep a record out + of the shared/committed store instead. Use when the user says "remember this", "store this", "save to memory", "index these", "eidetic remember", or when something learned this session should outlive it. Pairs with the sibling /recall skill. @@ -62,11 +64,12 @@ The wrapper resolves the CLI portably (installed `eidetic` on `PATH`, else | `links` | optional | list of related-memory ids; persisted for future corroboration scoring | `score` and `signal` are recall-only and are ignored on ingest. **Mind the -scope:** the default personal scope is **private** (`--scope eidetic-cli ---visibility private`), so personal/role-gated notes stay isolated to this -agent's recall and are safe to store. Only when you deliberately write to a -**public** scope (`--visibility public`) does the record enter the shared pool -visible to every scope — keep public-scope records to public data only. +scope:** the default personal scope is **public** (`--scope eidetic-cli +--visibility public`), so a plain remember is team-shared and travels with +the repo (`/.eidetic/memory`, committed) — keep default-scope +records to shareable data. Only when you deliberately write to a **private** +scope (`--visibility private`) is the record isolated to `$HOME/.eidetic/memory` +and invisible to any other scope's recall (including a public one). ## Idempotency @@ -93,12 +96,12 @@ eidetic sweep # apply transitions - `--json` — structured result (`{"upserted": N, "ids": [...]}`) to stdout. - `--scope NAME` / `--visibility public|private` — record scope. **The wrapper - defaults this to the agent's PERSONAL, PRIVATE scope** — `--scope - --visibility private`, where `` is read from the nearest `culture.yaml` - (here, `eidetic-cli`). Private records are served only to a recall in the same - scope, so they don't leak to a `default`/other-scope query. Pass `--scope` to - steer to a different scope (which then uses the plain CLI default visibility), - or `--visibility public` to keep the personal scope but make it shared. A wheel + defaults this to the agent's PERSONAL, PUBLIC scope** — `--scope + --visibility public`, where `` is read from the nearest `culture.yaml` + (here, `eidetic-cli`). Public records are visible to any scope's recall (no + isolation). Pass `--scope` to steer to a different scope (which then uses + the plain CLI default visibility, also `public`), or `--visibility private` + to keep the personal scope but isolate it to `$HOME/.eidetic/memory`. A wheel install with no `culture.yaml` falls back to the CLI default `default`/`public`. - `--backend files|mongo|neo4j` — default `files` (the shared home-dir store); use `mongo`/`neo4j` (with `EIDETIC_MONGO_URI` / `NEO4J_URI`) for a server store. @@ -113,6 +116,10 @@ eidetic sweep # apply transitions `PATH` (in a dev checkout it isn't); the wrapper resolves it (`PATH` first, else `uv run eidetic`). For the docs, run `eidetic explain remember` if installed, otherwise `uv run --project eidetic explain remember`. +- **Scope + visibility convention:** see [`docs/contract.md`](../../../docs/contract.md) + (memory scope+visibility convention, v1) for the naming/default rules this + wrapper (and every other `eidetic remember`/`recall` consumer, including + the colleague backend's `colleague/memory.py`) pins to. ## Provenance diff --git a/.claude/skills/remember/scripts/remember.sh b/.claude/skills/remember/scripts/remember.sh index c54a94e..08f4735 100755 --- a/.claude/skills/remember/scripts/remember.sh +++ b/.claude/skills/remember/scripts/remember.sh @@ -59,10 +59,10 @@ Usage: A record needs `id`, `text`, and `type`; `hash` and `metadata` are recommended (hash is derived from text when omitted). Upsert is idempotent by id. -Records default to this agent's PRIVATE personal scope (--scope from the -culture.yaml suffix); pass --visibility public to contribute to the shared -public pool. Every flag is forwarded verbatim to `eidetic remember`. -See `eidetic explain remember`. +Records default to this agent's PERSONAL, PUBLIC scope (--scope from the +culture.yaml suffix, --visibility public); pass --visibility private to keep +a record out of the shared/committed store. Every flag is forwarded verbatim +to `eidetic remember`. See `eidetic explain remember`. EOF } @@ -85,7 +85,7 @@ fi resolve_eidetic || exit 2 -# ── default to this agent's PERSONAL, PRIVATE scope (culture.yaml `suffix`) ── +# ── default to this agent's PERSONAL, PUBLIC scope (culture.yaml `suffix`) ─── # A record this agent remembers should land in its OWN personal scope, not the # global `default` scope shared by every project on this host. We read the # `suffix` from the nearest culture.yaml (walking up from this script), so the @@ -94,14 +94,20 @@ resolve_eidetic || exit 2 # (running in a worktree of this same repo) resolves the same suffix, keeping # the Claude↔colleague shared-memory story intact. # -# The personal scope is PRIVATE by default: in eidetic's model only a private -# record is isolated to its scope (`can_serve`), so private is what actually -# keeps these records from leaking to a default/other-scope recall. Scope and -# visibility are paired — the private default applies only when we inject the -# resolved scope, and only if the caller didn't pass --visibility (so an -# explicit `--visibility public` still wins). An explicit --scope on the command -# line takes over steering entirely; a wheel install with no culture.yaml falls -# back to the plain CLI default (`default`/`public`). +# The personal scope is PUBLIC by default — the memory scope+visibility +# convention (v1, docs/contract.md): an in-repo record is team-shared by +# default (routing already sends a public write inside a git repo to +# /.eidetic/memory, committed alongside the code), matching both +# the plain `eidetic remember` CLI's own --visibility default and the +# colleague backend's runtime (colleague/memory.py hardcodes --visibility +# public) — so a no-flag remember here and a no-flag `eidetic remember` +# elsewhere land mutually-visible records instead of silently disagreeing. +# Scope and visibility are paired — the public default applies only when we +# inject the resolved scope, and only if the caller didn't pass --visibility +# (so an explicit `--visibility private` still wins, keeping genuinely +# personal/non-shareable data out of the committed store). An explicit +# --scope on the command line takes over steering entirely; a wheel install +# with no culture.yaml falls back to the plain CLI default (`default`/`public`). resolve_scope() { local dir suffix="" dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) @@ -141,16 +147,15 @@ 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) - elif ! has_flag --visibility "$@"; then - # No suffix AND no explicit --visibility: the record falls back to - # eidetic's own default (scope=default, visibility=public). Don't let an - # expected-private record go public silently — warn on stderr (stdout - # stays clean for --json). Warn ONLY here: an explicit --scope (outer - # guard) or --visibility (this guard) means the caller chose deliberately - # and is honored verbatim, so either flag silences this. - printf 'warning: no culture.yaml suffix resolved; this record falls back to the public default scope. Pass --scope or --visibility to place it deliberately.\n' >&2 + has_flag --visibility "$@" || SCOPE_ARGS+=(--visibility public) fi + # No suffix resolved (e.g. a wheel install with no culture.yaml): leave + # --scope/--visibility unset entirely, so the plain `eidetic remember` + # defaults apply (scope=default, visibility=public) — identical + # visibility to the suffix-resolved case above, just grouped under the + # `default` scope name instead of this agent's personal one. Nothing to + # warn about: unlike the old private-by-default behavior, there is no + # privacy downgrade here either way. fi : "${EIDETIC_EMBED_URL:=http://localhost:8002/v1}" diff --git a/docs/contract.md b/docs/contract.md new file mode 100644 index 0000000..5382837 --- /dev/null +++ b/docs/contract.md @@ -0,0 +1,127 @@ +# Memory scope + visibility convention (v1) + +One convention every consumer of the eidetic memory store — the vendored +`recall`/`remember` skill wrappers in this repo, a bespoke runtime +integration like the colleague backend's `colleague/memory.py`, or a future +direct CLI consumer — pins to, so that mutual visibility between agents and +backends is a documented guarantee rather than an accident of whichever +hardcoded flags happen to agree today. + +This document does not change any eidetic Python code, record schema, +routing, or CLI behavior. It only states, in one place, the convention that +`recall.sh` / `remember.sh` (this repo's vendored skill wrappers) are fixed +to follow in this same change, and that the plain `eidetic remember` / +`eidetic recall` CLI already followed before this change (its own +`--visibility` argparse default is `"public"` — see +`eidetic/cli/_commands/remember.py` and `eidetic/cli/_commands/recall.py`). + +## Machine-readable summary + +The block below is parsed by `tests/test_contract_drift.py` and compared +against the wrapper scripts' text and the CLI's actual argparse default. +Keep it in sync with both — a future divergence on either side fails that +test. + +```text +version: 1 +default_visibility: public +private_requires_explicit_flag: true +scope_naming: culture.yaml-suffix-per-repo +``` + +## 1. Scope naming + +**One agent-personal scope per repo, named by that repo's `culture.yaml` +top-level agent `suffix`** — the same identity `eidetic whoami` reports. +This is a *naming* convention layered on top of the existing `Scope` +primitive (`name: str`, `visibility: "public" | "private"`, +`eidetic/memory/scope.py`); the primitive itself is unchanged. + +Record **families** — `task`, `experiment`, `lesson`, and so on — are **not** +separate scope primitives and this convention does not add any. They ride +the existing `Record.type` field (a free-form string) plus `metadata` +facets, e.g. `{"type": "lesson", "metadata": {"family": "lesson", ...}}`. +Don't invent a new scope kind to express "this is a lesson, not a task" — +that distinction belongs to `type`/`metadata`, which already round-trip +verbatim through ingest and recall. + +## 2. Visibility default + +**Default: `public`, for in-repo team-shared records.** Store routing +already sends a public write made inside a git repo to +`/.eidetic/memory` — committed alongside the code, so it travels +with a `git clone` / `git worktree add` and is visible to every agent or +backend that later checks out the repo. That is the behavior public-by- +default is *for*. + +`private` stays reachable, one explicit `--visibility private` away, for +data that genuinely shouldn't travel with the repo: it resolves to +`$HOME/.eidetic/memory` (never committed) and is invisible to any other +scope's recall — including a public one (`Scope.can_serve`, +`eidetic/memory/scope.py`: a private record is served only to a query in +the *same* scope; a public record is served to any query). + +This was already the plain CLI's own default. Before this convention was +written down, this repo's own vendored wrapper scripts silently +*overrode* that default to `private` for a no-flag invocation — while +colleague's separate `colleague/memory.py` runtime hardcodes `public` for +its own shell-outs. Two consumers of the same store, disagreeing on the +default, by accident rather than decision. v1 makes the decision explicit: +**public wins**, and the wrappers in this repo are fixed to match (see +[History](#history--versioning) below for why this is a deliberate +reversal, not an oversight). + +## 3. Store-resolution table + +Copied from `eidetic/memory/backend.py` (`_resolve_write_dir` / +`_candidate_read_dirs`) — this is the actual code path, not a paraphrase: + +| Precedence | Condition | Resolves to | +|---|---|---| +| 1 | `EIDETIC_DATA_DIR` set | that directory, unconditionally — both reads and writes, regardless of visibility | +| 2 | `visibility == "public"` **and** inside a git repo | `/.eidetic/memory` (write target); reads merge this with row 3 | +| 3 | otherwise — `private`, or outside any git repo | `$HOME/.eidetic/memory` | + +A read (`eidetic recall`) merges `$HOME/.eidetic/memory` with the repo store +whenever one exists and no override is set, then applies `Scope.can_serve` +per candidate record: a `public`-visibility query sees every public record +(any scope name) and no private record at all; a `private`-visibility query +sees every public record *plus* private records that exactly match its own +scope name. (Verified live, 2026-07: a `public` query never returns a +private record regardless of scope name — `can_serve`'s private branch can +only match when the query's own visibility is also `private`.) + +## 4. For consumers + +Any process that shells out to `eidetic remember` / `eidetic recall` — +whether through these vendored skill wrappers or a bespoke runtime +integration such as colleague's `colleague/memory.py` — should pin to this +convention with its **own** drift test, rather than trusting that its +hardcoded scope/visibility flags happen to match this document or each +other. This repo's drift test (`tests/test_contract_drift.py`) can't +literally be reused cross-repo, but the pattern is: parse this file's +machine-readable block, then assert it against your own hardcoded flags (or +argparse defaults) so a future edit on either side is caught instead of +silently drifting back into disagreement. + +## History / versioning + +- **v1** (this document — [eidetic-cli#28](https://github.com/agentculture/eidetic-cli/issues/28) + / [colleague#291](https://github.com/agentculture/colleague/issues/291) S9): + default visibility declared `public` across every surface (the CLI, the + vendored wrappers, and colleague's runtime). This is a **deliberate + reversal** of the `FIX-4` decision recorded in + [`docs/specs/2026-06-23-eidetic-cli-s-remember-recall-skill-wrappers-are-h.md`](specs/2026-06-23-eidetic-cli-s-remember-recall-skill-wrappers-are-h.md), + which affirmed that a `private` personal-scope default was intentional at + the time and only fixed the doc/behavior *mismatch* around it (the + wrapper's help text used to say "Public data only" while defaulting to + private — that spec made the docs honest about the private default; it + did not question whether private was the right default). That decision + predates the cross-repo mutual-visibility requirement colleague#291 + surfaced. It is recorded here as a visible, deliberate convention change — + not a silent breach of that earlier decision. +- Future revisions to this convention (scope naming, the visibility default, + or the store-resolution table) should bump the `version` field in the + machine-readable block above and add a dated entry here, so + `tests/test_contract_drift.py` and a human reader both notice a version + bump that isn't matched by a doc update (or vice versa). diff --git a/tests/test_contract_drift.py b/tests/test_contract_drift.py new file mode 100644 index 0000000..df36973 --- /dev/null +++ b/tests/test_contract_drift.py @@ -0,0 +1,113 @@ +"""Drift test for the memory scope+visibility convention (v1, docs/contract.md). + +Three surfaces state the SAME default visibility today: the convention +document, the two vendored skill wrapper scripts, and the plain `eidetic` +CLI's own argparse default. Nothing keeps them in sync automatically — this +test is that guard. It parses the machine-readable block in +``docs/contract.md``, then compares it against: + +1. the literal default `--visibility` value each wrapper script injects + when the caller passes neither `--scope` nor `--visibility` (a text + check on the committed `.sh` files, not a subprocess run — this test + fails fast even if bash/uv aren't on PATH), and +2. the CLI's ACTUAL argparse default for `--visibility` on both `remember` + and `recall`, obtained by exercising the real `register()` + `parse_args` + path (not by re-implementing argparse introspection) so a future refactor + of the CLI's default is caught here even if nobody thinks to update this + test. + +A future edit that changes any ONE of these three without updating the +others fails this test. +""" + +from __future__ import annotations + +import argparse +import re +from pathlib import Path + +import pytest + +from eidetic.cli._commands import recall as recall_cmd +from eidetic.cli._commands import remember as remember_cmd + +REPO_ROOT = Path(__file__).resolve().parents[1] +CONTRACT = REPO_ROOT / "docs" / "contract.md" +REMEMBER_SH = REPO_ROOT / ".claude" / "skills" / "remember" / "scripts" / "remember.sh" +RECALL_SH = REPO_ROOT / ".claude" / "skills" / "recall" / "scripts" / "recall.sh" + + +def _parse_contract_block(text: str) -> dict[str, str]: + """Parse the fenced ```text machine-readable block in docs/contract.md. + + Deliberately NOT a YAML parse (eidetic keeps zero runtime deps and reads + culture.yaml the same hand-rolled way, per + ``eidetic/cli/_commands/whoami.py``'s ``read_agent_fields`` docstring) — + a simple ``key: value`` line parse is all the block needs. + """ + match = re.search(r"```text\n(.*?)\n```", text, re.DOTALL) + assert match is not None, "docs/contract.md is missing its ```text machine-readable block" + fields: dict[str, str] = {} + for line in match.group(1).splitlines(): + line = line.strip() + if not line or ":" not in line: + continue + key, _, value = line.partition(":") + fields[key.strip()] = value.strip() + return fields + + +@pytest.fixture(scope="module") +def contract() -> dict[str, str]: + text = CONTRACT.read_text(encoding="utf-8") + return _parse_contract_block(text) + + +def _wrapper_default_visibility(script_path: Path) -> str: + """Extract the literal default `--visibility` value a wrapper injects. + + Looks for the one `SCOPE_ARGS+=(--visibility )` line inside the + "no --scope, no --visibility passed, but a culture.yaml suffix resolved" + branch — the site that decides what an agent gets when it doesn't pass + either flag. + """ + text = script_path.read_text(encoding="utf-8") + match = re.search(r"SCOPE_ARGS\+=\(--visibility (public|private)\)", text) + assert match is not None, f"{script_path} has no SCOPE_ARGS+=(--visibility ...) default line" + return match.group(1) + + +def _cli_default_visibility(register) -> str: + """Exercise the real argparse registration + parsing path to obtain the + CLI's actual runtime default for --visibility (not a re-implementation of + argparse's own introspection — the real parse_args() call).""" + parser = argparse.ArgumentParser() + sub = parser.add_subparsers() + register(sub) + # `remember` takes a positional record; `recall` takes a positional query. + # Either accepts an arbitrary string in that slot without touching + # --visibility, which is exactly what we want to leave at its default. + verb = sub.choices and next(iter(sub.choices)) + ns = parser.parse_args([verb, "{}"]) + return ns.visibility + + +def test_contract_declares_public_default(contract: dict[str, str]) -> None: + assert contract.get("default_visibility") == "public" + assert contract.get("private_requires_explicit_flag") == "true" + + +def test_remember_wrapper_default_matches_contract(contract: dict[str, str]) -> None: + assert _wrapper_default_visibility(REMEMBER_SH) == contract["default_visibility"] + + +def test_recall_wrapper_default_matches_contract(contract: dict[str, str]) -> None: + assert _wrapper_default_visibility(RECALL_SH) == contract["default_visibility"] + + +def test_remember_cli_actual_default_matches_contract(contract: dict[str, str]) -> None: + assert _cli_default_visibility(remember_cmd.register) == contract["default_visibility"] + + +def test_recall_cli_actual_default_matches_contract(contract: dict[str, str]) -> None: + assert _cli_default_visibility(recall_cmd.register) == contract["default_visibility"] diff --git a/tests/test_wrapper_convention.py b/tests/test_wrapper_convention.py new file mode 100644 index 0000000..ba2c608 --- /dev/null +++ b/tests/test_wrapper_convention.py @@ -0,0 +1,199 @@ +"""Integration test for the memory scope+visibility convention (v1). + +See ``docs/contract.md``. The convention says the DEFAULT visibility for a +no-flag `remember`/`recall` is `public` — matching the plain `eidetic` +CLI's own argparse default AND colleague's runtime (`colleague/memory.py` +hardcodes `--visibility public`). + +Before this convention was pinned, the vendored `remember.sh` wrapper +silently overrode that default to `private` (injecting `--scope +--visibility private` whenever the caller passed neither flag), while the +plain `eidetic remember` CLI defaulted to `public` — so an agent going +through the wrapper and an agent (or the colleague backend) calling the raw +CLI directly stored MUTUALLY INVISIBLE records under the same repo, purely +by accident of which surface they happened to use. + +This test shells both surfaces — the vendored wrapper script and the raw +`eidetic` CLI (via ``python -m eidetic``, the same interpreter running +pytest so no extra `uv run` subprocess/network is needed) — with no +`--visibility` flag, points `EIDETIC_DATA_DIR` at a throwaway directory so +nothing touches a real store, and asserts both land a record with the SAME +visibility (`public`). It fails before the wrapper fix (wrapper writes +`private`, raw CLI writes `public`) and passes after it. A follow-up +assertion proves the practical consequence: a plain no-flag `eidetic +recall` (the CLI's OWN default, scope=default/visibility=public) finds +both records — i.e. the two surfaces are mutually visible. +""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[1] +REMEMBER_WRAPPER = REPO_ROOT / ".claude" / "skills" / "remember" / "scripts" / "remember.sh" +RECALL_WRAPPER = REPO_ROOT / ".claude" / "skills" / "recall" / "scripts" / "recall.sh" + +pytestmark = pytest.mark.skipif( + not (REMEMBER_WRAPPER.exists() and RECALL_WRAPPER.exists()), + reason="vendored recall/remember wrappers not present in this checkout", +) + + +def _run(argv: list[str], *, cwd: Path, env: dict) -> subprocess.CompletedProcess: + return subprocess.run( + argv, + cwd=str(cwd), + env=env, + capture_output=True, + text=True, + timeout=60, + ) + + +def _base_env(data_dir: Path) -> dict: + """A minimal, isolated environment: EIDETIC_DATA_DIR pinned to a throwaway + dir so this test never touches a real store, and no live embed server + dependency (ingest never needs one; keyword recall doesn't either).""" + env = dict(os.environ) + env["EIDETIC_DATA_DIR"] = str(data_dir) + env.pop("EIDETIC_EMBED_URL", None) + env.pop("EIDETIC_EMBED_MODEL", None) + return env + + +def _raw_eidetic(*args: str) -> list[str]: + """Invoke the raw `eidetic` CLI via the current interpreter (no wrapper, + no scope/visibility injection) — the "unaware direct consumer" surface.""" + return [sys.executable, "-m", "eidetic", *args] + + +def _all_records(data_dir: Path) -> list[dict]: + """Read every record from every *.jsonl file under data_dir (any scope).""" + records: list[dict] = [] + if not data_dir.exists(): + return records + for f in data_dir.glob("*.jsonl"): + for line in f.read_text(encoding="utf-8").splitlines(): + line = line.strip() + if line: + records.append(json.loads(line)) + return records + + +def _visibility_of(records: list[dict], record_id: str) -> str | None: + for r in records: + if r["id"] == record_id: + return r["scope"]["visibility"] + return None + + +def test_wrapper_and_raw_cli_default_to_the_same_visibility(tmp_path: Path) -> None: + data_dir = tmp_path / "store" + env = _base_env(data_dir) + + wrapper_record = { + "id": "wrap-conv-1", + "text": "wrapper convention record", + "type": "note", + } + raw_record = { + "id": "raw-conv-1", + "text": "raw cli convention record", + "type": "note", + } + + # The vendored wrapper: no --scope, no --visibility. It resolves the + # culture.yaml suffix itself (walking up from the script's own location), + # so it doesn't matter that we run from the repo root — that just matches + # how an agent invokes it in practice. + wrapper_result = _run( + ["bash", str(REMEMBER_WRAPPER), json.dumps(wrapper_record), "--json"], + cwd=REPO_ROOT, + env=env, + ) + assert wrapper_result.returncode == 0, wrapper_result.stderr + + # The raw CLI, exactly as an unaware direct consumer would call it: no + # --scope, no --visibility. + raw_result = _run( + _raw_eidetic("remember", json.dumps(raw_record), "--json"), + cwd=REPO_ROOT, + env=env, + ) + assert raw_result.returncode == 0, raw_result.stderr + + records = _all_records(data_dir) + wrapper_visibility = _visibility_of(records, "wrap-conv-1") + raw_visibility = _visibility_of(records, "raw-conv-1") + + assert wrapper_visibility is not None, "wrapper record was not written" + assert raw_visibility is not None, "raw CLI record was not written" + + # THE CONVENTION (docs/contract.md): both default to the SAME visibility. + # This assertion fails before the wrapper fix (wrapper writes "private", + # raw CLI writes "public") and passes after it. + assert wrapper_visibility == raw_visibility == "public" + + # Prove the practical consequence: a plain no-flag `eidetic recall` (the + # CLI's OWN default — scope=default, visibility=public) finds BOTH + # records, i.e. the wrapper's record is no longer invisible to a caller + # that doesn't know about the wrapper's scope-injection at all. + recall_result = _run( + _raw_eidetic("recall", "convention record", "--mode", "keyword", "--top-k", "10", "--json"), + cwd=REPO_ROOT, + env=env, + ) + assert recall_result.returncode == 0, recall_result.stderr + hits = json.loads(recall_result.stdout) + ids = {h["id"] for h in hits} + assert "wrap-conv-1" in ids, "wrapper's record is invisible to a plain public recall" + assert "raw-conv-1" in ids + + +def test_recall_wrapper_sees_remember_wrapper_records_with_no_flags(tmp_path: Path) -> None: + """The two wrapper scripts must round-trip with no flags on either side: + a no-flag `remember.sh` write must be visible to a no-flag `recall.sh` + query. Both inject the same culture.yaml-suffix scope with the same + (now public) default visibility, so this holds regardless of whether the + query matches the write's scope name exactly (public records are visible + to any scope).""" + data_dir = tmp_path / "store" + env = _base_env(data_dir) + + record = { + "id": "wrap-roundtrip-1", + "text": "roundtrip through both wrappers", + "type": "note", + } + + remember_result = _run( + ["bash", str(REMEMBER_WRAPPER), json.dumps(record), "--json"], + cwd=REPO_ROOT, + env=env, + ) + assert remember_result.returncode == 0, remember_result.stderr + + recall_result = _run( + [ + "bash", + str(RECALL_WRAPPER), + "roundtrip through both wrappers", + "--mode", + "keyword", + "--top-k", + "5", + "--json", + ], + cwd=REPO_ROOT, + env=env, + ) + assert recall_result.returncode == 0, recall_result.stderr + hits = json.loads(recall_result.stdout) + ids = {h["id"] for h in hits} + assert "wrap-roundtrip-1" in ids From 82b2b88353a18a008a4cded417f88dc35e16faf7 Mon Sep 17 00:00:00 2001 From: Ori Nachum Date: Mon, 6 Jul 2026 21:29:52 +0300 Subject: [PATCH 2/4] t20(291/S2): embedder default aligned to the documented reference (8002 / Qwen3-Embedding-0.6B), drift-tested (#28) Co-Authored-By: Claude Fable 5 --- .claude/skills/recall/scripts/recall.sh | 4 ++ .claude/skills/remember/scripts/remember.sh | 4 ++ README.md | 20 ++++++ docs/contract.md | 57 +++++++++++++++ eidetic/memory/embed.py | 12 +++- tests/test_embed_default_drift.py | 79 +++++++++++++++++++++ 6 files changed, 174 insertions(+), 2 deletions(-) create mode 100644 tests/test_embed_default_drift.py diff --git a/.claude/skills/recall/scripts/recall.sh b/.claude/skills/recall/scripts/recall.sh index 1f4ef7e..9e319f6 100755 --- a/.claude/skills/recall/scripts/recall.sh +++ b/.claude/skills/recall/scripts/recall.sh @@ -161,6 +161,10 @@ fi # Default the embedding endpoint to the local model-gear embed gear. eidetic # falls back to a deterministic offline embedding if it's unreachable, so this # is safe even when the gear is down. Override by exporting these yourself. +# As of eidetic-cli#28 (colleague#293) these values now match eidetic's own +# code default in eidetic/memory/embed.py, so this export is a redundant-but- +# harmless belt-and-suspenders default kept for explicitness and for older +# eidetic installs that predate the code-default alignment. : "${EIDETIC_EMBED_URL:=http://localhost:8002/v1}" : "${EIDETIC_EMBED_MODEL:=Qwen/Qwen3-Embedding-0.6B}" export EIDETIC_EMBED_URL EIDETIC_EMBED_MODEL diff --git a/.claude/skills/remember/scripts/remember.sh b/.claude/skills/remember/scripts/remember.sh index 08f4735..ec0d490 100755 --- a/.claude/skills/remember/scripts/remember.sh +++ b/.claude/skills/remember/scripts/remember.sh @@ -158,6 +158,10 @@ if ! has_flag --scope "$@"; then # privacy downgrade here either way. fi +# As of eidetic-cli#28 (colleague#293) these values now match eidetic's own +# code default in eidetic/memory/embed.py, so this export is a redundant-but- +# harmless belt-and-suspenders default kept for explicitness and for older +# eidetic installs that predate the code-default alignment. : "${EIDETIC_EMBED_URL:=http://localhost:8002/v1}" : "${EIDETIC_EMBED_MODEL:=Qwen/Qwen3-Embedding-0.6B}" export EIDETIC_EMBED_URL EIDETIC_EMBED_MODEL diff --git a/README.md b/README.md index dace3f5..99dd1b1 100644 --- a/README.md +++ b/README.md @@ -78,6 +78,26 @@ Embeddings + rerank come from a separate model-gear HTTP endpoint local lexical fallback when it is absent. Only `approximate`/`hybrid` recall use it; `exact`/`keyword` are pure lexical and work fully offline. +**Reference deployment.** The default above is the reference rig's real +embedder, and it is now a single agreed value across every surface — the +`eidetic/memory/embed.py` code default, the vendored `recall.sh`/`remember.sh` +wrapper exports, and this README (eidetic-cli#28 / colleague#293, +drift-tested by `tests/test_embed_default_drift.py`; see +[`docs/contract.md`](docs/contract.md) for the machine-readable record). On +the AgentCulture mesh, the same endpoint is discoverable rather than +hardcoded: the `lobes` gateway serves an `embedder` role in its +`GET /capabilities` response (lobes-cli >= 0.38, where each role's `endpoint` +field is client-reachable — see colleague's `colleague/lobes.py`), and a +consumer such as colleague resolves that role and injects +`EIDETIC_EMBED_URL`/`EIDETIC_EMBED_MODEL` into the environment before shelling +out to `eidetic`. That injection never overrides an operator's own explicit +export: `EmbedClient.__init__` (`eidetic/memory/embed.py`) reads +`os.environ.get("EIDETIC_EMBED_URL"/"EIDETIC_EMBED_MODEL")` ahead of the +built-in default and no CLI flag intervenes between the two, so an +explicitly-set env var always wins — eidetic's own env-first resolution +order already guarantees this; lobes discovery just supplies a smarter +default for the same environment slot, never a competing one. + ## Freshness signal Every record carries temporal state used to compute a *freshness signal* — a diff --git a/docs/contract.md b/docs/contract.md index 5382837..b48040b 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -104,6 +104,53 @@ machine-readable block, then assert it against your own hardcoded flags (or argparse defaults) so a future edit on either side is caught instead of silently drifting back into disagreement. +## 5. Embedder default (reference deployment) + +A second, independent alignment tracked under the same issue thread +([eidetic-cli#28](https://github.com/agentculture/eidetic-cli/issues/28) / +[colleague#293](https://github.com/agentculture/colleague/issues/293), +colleague#291 task S2): the embeddings/rerank endpoint default. Three +surfaces used to state it, and one of them — the `eidetic/memory/embed.py` +code default — disagreed with the other two (a stale +`http://localhost:8101/v1` + `text-embedding-3-small` pair, left over from +before the reference rig moved to its current embedder). The vendored +`recall.sh`/`remember.sh` wrappers and this repo's own README already +pointed at the real reference-rig endpoint, and every real invocation +exported that endpoint before the divergent code default ever got a chance +to matter — an accidental workaround, not a decision. This section states +the one, now-agreed value. + +```text +embed_default_url: http://localhost:8002/v1 +embed_default_model: Qwen/Qwen3-Embedding-0.6B +``` + +The block above is parsed by `tests/test_embed_default_drift.py` and +compared against `eidetic/memory/embed.py`'s `_DEFAULT_BASE_URL` / +`_DEFAULT_MODEL` module constants and the literal `:=` default each vendored +wrapper script exports. A future divergence on any of the three fails that +test. + +**On the AgentCulture mesh, this endpoint is discoverable, not just +hardcoded.** The `lobes` gateway serves an `embedder` role in its +`GET /capabilities` response (lobes-cli >= 0.38, where each role's +`endpoint` field is client-reachable — see colleague's `colleague/lobes.py` +docstring for the pre-0.38/post-0.38 distinction). A consumer such as +colleague resolves that role from lobes and injects +`EIDETIC_EMBED_URL`/`EIDETIC_EMBED_MODEL` into the process environment +before shelling out to `eidetic remember`/`eidetic recall` — the same +env-var seam the vendored wrappers use above, just populated from live +discovery instead of a fixed `:=` default. This never overrides an +operator's own explicit export: `EmbedClient.__init__` +(`eidetic/memory/embed.py`) reads +`os.environ.get("EIDETIC_EMBED_URL"/"EIDETIC_EMBED_MODEL")` ahead of the +`_DEFAULT_BASE_URL`/`_DEFAULT_MODEL` constants, and no CLI flag sits between +the two (`EmbedClient()` is constructed with no arguments at its one call +site, `eidetic/memory/backend.py`) — so an explicitly-set env var always +wins. eidetic's own env-first resolution order already guarantees this; +lobes discovery only ever supplies a smarter *default* for that same slot, +never a competing override path. + ## History / versioning - **v1** (this document — [eidetic-cli#28](https://github.com/agentculture/eidetic-cli/issues/28) @@ -125,3 +172,13 @@ silently drifting back into disagreement. machine-readable block above and add a dated entry here, so `tests/test_contract_drift.py` and a human reader both notice a version bump that isn't matched by a doc update (or vice versa). +- **S2** ([eidetic-cli#28](https://github.com/agentculture/eidetic-cli/issues/28) + / [colleague#293](https://github.com/agentculture/colleague/issues/293), + colleague#291 task S2): added [§5](#5-embedder-default-reference-deployment), + a separate embedder-endpoint-default alignment — the `eidetic/memory/embed.py` + code default moved to match the README and the vendored wrappers + (`http://localhost:8002/v1` / `Qwen/Qwen3-Embedding-0.6B`), plus the note on + how a `lobes`-gateway `embedder` role can supply that same default by live + discovery. This is unrelated to the scope+visibility convention's own + `version` field above; retrieval logic, fallback behavior, and timeouts are + unchanged. diff --git a/eidetic/memory/embed.py b/eidetic/memory/embed.py index 320516f..d8e4484 100644 --- a/eidetic/memory/embed.py +++ b/eidetic/memory/embed.py @@ -18,8 +18,16 @@ # Configuration # ----------------------------------------------------------------------- -_DEFAULT_BASE_URL = "http://localhost:8101/v1" -_DEFAULT_MODEL = "text-embedding-3-small" +# Aligned to the reference deployment documented in README.md and +# docs/contract.md (eidetic-cli#28 / colleague#293): the vendored +# `recall.sh`/`remember.sh` wrappers and this project's own README already +# pointed at the real embedder rig (localhost:8002, Qwen3-Embedding-0.6B) — +# these code defaults used to diverge from that (a stale localhost:8101 + +# text-embedding-3-small pair) and only matched by accident of every real +# invocation exporting the wrapper's override first. Now all three surfaces +# agree; drift-tested by tests/test_embed_default_drift.py. +_DEFAULT_BASE_URL = "http://localhost:8002/v1" +_DEFAULT_MODEL = "Qwen/Qwen3-Embedding-0.6B" _EMBED_DIM = 128 _EMBED_TIMEOUT: float = float(os.environ.get("EIDETIC_EMBED_TIMEOUT", "10")) diff --git a/tests/test_embed_default_drift.py b/tests/test_embed_default_drift.py new file mode 100644 index 0000000..1eb49a8 --- /dev/null +++ b/tests/test_embed_default_drift.py @@ -0,0 +1,79 @@ +"""Drift test for the embedder endpoint default (docs/contract.md §5). + +Three surfaces state the SAME embedder default today: `eidetic/memory/embed.py`'s +`_DEFAULT_BASE_URL`/`_DEFAULT_MODEL` code constants, the `docs/contract.md` +machine-readable block, and the vendored `recall.sh`/`remember.sh` skill +wrapper scripts' `:=` exports. Before eidetic-cli#28 (colleague#293) the code +constants silently disagreed with the other two (a stale +`http://localhost:8101/v1` + `text-embedding-3-small` pair) — this test is the +guard that keeps all three in sync going forward. A future edit that changes +any ONE of these three without updating the others fails this test. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +from eidetic.memory import embed + +REPO_ROOT = Path(__file__).resolve().parents[1] +CONTRACT = REPO_ROOT / "docs" / "contract.md" +REMEMBER_SH = REPO_ROOT / ".claude" / "skills" / "remember" / "scripts" / "remember.sh" +RECALL_SH = REPO_ROOT / ".claude" / "skills" / "recall" / "scripts" / "recall.sh" + + +def _parse_embed_block(text: str) -> dict[str, str]: + """Parse the ```text machine-readable block that carries `embed_default_*` + keys in docs/contract.md. + + docs/contract.md has more than one ```text fenced block (the scope+ + visibility convention's own block comes first), so this scans every block + and returns the first one containing an `embed_default_url` key — rather + than assuming block order — following the same hand-rolled `key: value` + line parse `tests/test_contract_drift.py` uses (deliberately not YAML; + eidetic keeps zero runtime deps). + """ + for match in re.finditer(r"```text\n(.*?)\n```", text, re.DOTALL): + fields: dict[str, str] = {} + for line in match.group(1).splitlines(): + line = line.strip() + if not line or ":" not in line: + continue + key, _, value = line.partition(":") + fields[key.strip()] = value.strip() + if "embed_default_url" in fields: + return fields + raise AssertionError("docs/contract.md is missing its embed_default_url machine-readable block") + + +def _wrapper_default(script_path: Path, var_name: str) -> str: + """Extract the literal `:=` default a wrapper exports for *var_name*.""" + text = script_path.read_text(encoding="utf-8") + match = re.search(r':\s*"\$\{' + re.escape(var_name) + r':=([^}]+)\}"', text) + assert match is not None, f"{script_path} has no {var_name}:=... default line" + return match.group(1) + + +def test_contract_declares_embed_defaults() -> None: + fields = _parse_embed_block(CONTRACT.read_text(encoding="utf-8")) + assert fields["embed_default_url"] == "http://localhost:8002/v1" + assert fields["embed_default_model"] == "Qwen/Qwen3-Embedding-0.6B" + + +def test_embed_py_code_default_matches_contract() -> None: + fields = _parse_embed_block(CONTRACT.read_text(encoding="utf-8")) + assert embed._DEFAULT_BASE_URL == fields["embed_default_url"] + assert embed._DEFAULT_MODEL == fields["embed_default_model"] + + +def test_recall_wrapper_default_matches_contract() -> None: + fields = _parse_embed_block(CONTRACT.read_text(encoding="utf-8")) + assert _wrapper_default(RECALL_SH, "EIDETIC_EMBED_URL") == fields["embed_default_url"] + assert _wrapper_default(RECALL_SH, "EIDETIC_EMBED_MODEL") == fields["embed_default_model"] + + +def test_remember_wrapper_default_matches_contract() -> None: + fields = _parse_embed_block(CONTRACT.read_text(encoding="utf-8")) + assert _wrapper_default(REMEMBER_SH, "EIDETIC_EMBED_URL") == fields["embed_default_url"] + assert _wrapper_default(REMEMBER_SH, "EIDETIC_EMBED_MODEL") == fields["embed_default_model"] From 33ebd17f2a07b29be2c035d546badb7a35bbf34f Mon Sep 17 00:00:00 2001 From: Ori Nachum Date: Mon, 6 Jul 2026 22:47:12 +0300 Subject: [PATCH 3/4] chore: bump to 0.11.0 (memory scope+visibility contract v1 + embedder default alignment) (#28) Record the CHANGELOG entry for docs/contract.md (memory scope+visibility convention v1), the recall/remember wrapper default reversal to public visibility, and the embedder default alignment to 8002/Qwen3-Embedding-0.6B. --- CHANGELOG.md | 31 +++++++++++++++++++++++++++++++ pyproject.toml | 2 +- uv.lock | 2 +- 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fd088c..47327c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,37 @@ All notable changes to this project will be documented in this file. Format follows [Keep a Changelog](https://keepachangelog.com/). This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.11.0] - 2026-07-06 + +### Added + +- `docs/contract.md` — the memory scope+visibility convention (v1) every + consumer of the eidetic memory store pins to: a personal scope per repo + named by `culture.yaml`'s `suffix`, and a `public` default visibility for + in-repo team-shared records (private stays one explicit + `--visibility private` away). Includes a machine-readable summary block + cross-checked by `tests/test_contract_drift.py`. + +### Changed + +- The vendored `recall`/`remember` skill wrappers + (`.claude/skills/{recall,remember}/scripts/*.sh`) now default to `public` + visibility for a no-flag invocation, matching the plain `eidetic + remember`/`recall` CLI's own argparse default (already `public`, unchanged) + and colleague's own `colleague/memory.py` runtime. This is a deliberate + **reversal** of the private-by-default decision recorded in + `docs/specs/2026-06-23-eidetic-cli-s-remember-recall-skill-wrappers-are-h.md` + (FIX-4) — recorded honestly in `docs/contract.md`'s History section rather + than silently overwritten. **Downstream note:** these wrappers are + fanned out to roughly 57 consumer repos (including colleague's own + vendored copy); this fix needs a downstream re-sync/rollout to take + effect there. +- The embedder code default is aligned to the documented reference + deployment (port 8002 / `Qwen3-Embedding-0.6B`, was 8101 / + `text-embedding-3-small`), with a three-way drift test + (`tests/test_embed_default_drift.py`) pinning code, docs, and the wrapper + scripts in agreement. + ## [0.10.1] - 2026-06-24 ### Fixed diff --git a/pyproject.toml b/pyproject.toml index 01bedae..a0b83be 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "eidetic-cli" -version = "0.10.1" +version = "0.11.0" description = "Agent/CLI providing eidetic perfect-recall memory" readme = "README.md" license = "MIT" diff --git a/uv.lock b/uv.lock index 18ea7e0..126e70a 100644 --- a/uv.lock +++ b/uv.lock @@ -180,7 +180,7 @@ wheels = [ [[package]] name = "eidetic-cli" -version = "0.10.0" +version = "0.11.0" source = { editable = "." } dependencies = [ { name = "data-refinery-cli", extra = ["store"] }, From 2b5716079f3e6553b0188276d8d4a8b823e3f596 Mon Sep 17 00:00:00 2001 From: Ori Nachum Date: Tue, 7 Jul 2026 00:40:08 +0300 Subject: [PATCH 4/4] test(wrapper-convention): address Qodo PR #29 review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Harden tests/test_wrapper_convention.py — test-only, no wrapper change: - Finding 1 (store resolution): new hermetic test test_wrapper_and_raw_cli_resolve_the_same_default_store_location proves both surfaces resolve the SAME store dir under the real _resolve_write_dir algorithm (throwaway `git init` repo + redirected HOME, no EIDETIC_DATA_DIR override). Safe because store-dir resolution is CWD-driven, independent of the wrapper's script-location scope resolution. (Qodo comment 3531674260) - Finding 2 (version skew): _pin_eidetic_on_path prepends the venv bin to PATH so the wrapper's `command -v eidetic` resolves to this checkout's 0.11.0 console script, not a globally-installed eidetic (verified: a stale global 0.10.1 shadowed it before). No edit to the 57-repo-fanned-out wrappers. (Qodo comment 3531674266) - Finding 3 (file scraping): _all_records now reads via the in-process API get_backend("files").all() / record.scope.visibility instead of parsing *.jsonl, decoupling from data-refinery's on-disk layout. (Qodo comment 3531674270) Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 13 ++++ tests/test_wrapper_convention.py | 129 +++++++++++++++++++++++++++---- 2 files changed, 125 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76a44ab..0e28d23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,19 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). (`tests/test_embed_default_drift.py`) pinning code, docs, and the wrapper scripts in agreement. +### Fixed + +- Hardened `tests/test_wrapper_convention.py` per Qodo PR #29 review: it now + (1) proves wrapper and raw CLI resolve the *same default store location* + under the real algorithm — a throwaway `git init` repo + redirected `HOME`, + no `EIDETIC_DATA_DIR` short-circuit (finding 1); (2) pins the wrapper's + `command -v eidetic` to this checkout's console script so a globally-installed + `eidetic` on `PATH` can't make the two surfaces run different versions + (finding 2); and (3) reads records via the in-process backend API + (`get_backend("files").all()`) instead of scraping `*.jsonl`, decoupling from + data-refinery's on-disk layout (finding 3). Test-only — the fanned-out + wrappers are unchanged. + ## [0.10.2] - 2026-06-24 ### Added diff --git a/tests/test_wrapper_convention.py b/tests/test_wrapper_convention.py index ba2c608..541b221 100644 --- a/tests/test_wrapper_convention.py +++ b/tests/test_wrapper_convention.py @@ -35,6 +35,9 @@ import pytest +from eidetic.memory.backend import get_backend +from eidetic.memory.record import Record + REPO_ROOT = Path(__file__).resolve().parents[1] REMEMBER_WRAPPER = REPO_ROOT / ".claude" / "skills" / "remember" / "scripts" / "remember.sh" RECALL_WRAPPER = REPO_ROOT / ".claude" / "skills" / "recall" / "scripts" / "recall.sh" @@ -56,6 +59,19 @@ def _run(argv: list[str], *, cwd: Path, env: dict) -> subprocess.CompletedProces ) +def _pin_eidetic_on_path(env: dict) -> dict: + """Pin the wrapper's ``command -v eidetic`` resolution to THIS checkout's + console script (the same code as ``python -m eidetic``), by prepending the + directory of the running interpreter — under ``uv run`` the venv's + ``eidetic`` console script lives alongside its ``python``. Without this, a + globally-installed ``eidetic`` earlier on PATH would make the wrapper + subprocess and the raw-CLI comparison run DIFFERENT versions (Qodo PR #29 + finding 2). Mutates and returns *env*.""" + venv_bin = str(Path(sys.executable).parent) + env["PATH"] = venv_bin + os.pathsep + env.get("PATH", "") + return env + + def _base_env(data_dir: Path) -> dict: """A minimal, isolated environment: EIDETIC_DATA_DIR pinned to a throwaway dir so this test never touches a real store, and no live embed server @@ -64,7 +80,7 @@ def _base_env(data_dir: Path) -> dict: env["EIDETIC_DATA_DIR"] = str(data_dir) env.pop("EIDETIC_EMBED_URL", None) env.pop("EIDETIC_EMBED_MODEL", None) - return env + return _pin_eidetic_on_path(env) def _raw_eidetic(*args: str) -> list[str]: @@ -73,23 +89,27 @@ def _raw_eidetic(*args: str) -> list[str]: return [sys.executable, "-m", "eidetic", *args] -def _all_records(data_dir: Path) -> list[dict]: - """Read every record from every *.jsonl file under data_dir (any scope).""" - records: list[dict] = [] - if not data_dir.exists(): - return records - for f in data_dir.glob("*.jsonl"): - for line in f.read_text(encoding="utf-8").splitlines(): - line = line.strip() - if line: - records.append(json.loads(line)) - return records - - -def _visibility_of(records: list[dict], record_id: str) -> str | None: +def _all_records(data_dir: Path) -> list[Record]: + """Enumerate every record under *data_dir* via the in-process backend API + (``get_backend("files").all()``) instead of scraping ``*.jsonl`` directly — + so the test doesn't couple to data_refinery's on-disk file-per-scope layout + (Qodo PR #29 finding 3). ``EIDETIC_DATA_DIR`` is set as an override (matching + what the subprocess writes were given via ``_base_env``), then restored.""" + prior = os.environ.get("EIDETIC_DATA_DIR") + os.environ["EIDETIC_DATA_DIR"] = str(data_dir) + try: + return get_backend("files").all() + finally: + if prior is None: + os.environ.pop("EIDETIC_DATA_DIR", None) + else: + os.environ["EIDETIC_DATA_DIR"] = prior + + +def _visibility_of(records: list[Record], record_id: str) -> str | None: for r in records: - if r["id"] == record_id: - return r["scope"]["visibility"] + if r.id == record_id: + return r.scope.visibility return None @@ -197,3 +217,78 @@ def test_recall_wrapper_sees_remember_wrapper_records_with_no_flags(tmp_path: Pa hits = json.loads(recall_result.stdout) ids = {h["id"] for h in hits} assert "wrap-roundtrip-1" in ids + + +def test_wrapper_and_raw_cli_resolve_the_same_default_store_location( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Prove wrapper and raw CLI resolve the SAME store directory under the + REAL default-resolution algorithm (``eidetic/memory/backend.py`` + ``_resolve_write_dir``) — not merely identically via an ``EIDETIC_DATA_DIR`` + override that short-circuits it (Qodo PR #29 finding 1 / compliance ID + 1345474). + + Hermetic: runs inside a throwaway ``git init`` repo under ``tmp_path`` with + ``HOME`` also redirected under ``tmp_path``, so BOTH the public-write + repo-root branch and the private/HOME fallback branch land under + ``tmp_path`` — never touching this checkout's real ``.eidetic/memory`` or + the developer's real ``$HOME``. Safe because store-directory resolution is + CWD-driven (``_git_toplevel()`` shells ``git rev-parse --show-toplevel`` + against ``os.getcwd()``) — a SEPARATE axis from the wrapper's own + scope/visibility resolution, which walks up from the wrapper SCRIPT's own + location to find ``culture.yaml``. Redirecting CWD therefore doesn't disturb + which scope the wrapper injects; it only redirects where records land. + """ + repo = tmp_path / "repo" + repo.mkdir() + subprocess.run(["git", "init", str(repo)], capture_output=True, check=True) + + monkeypatch.delenv("EIDETIC_DATA_DIR", raising=False) + monkeypatch.setenv("HOME", str(tmp_path / "home")) + monkeypatch.chdir(repo) + + from eidetic.memory import backend as be + + be._GIT_CACHE.clear() + + env = dict(os.environ) + env.pop("EIDETIC_EMBED_URL", None) + env.pop("EIDETIC_EMBED_MODEL", None) + _pin_eidetic_on_path(env) + + wrapper_record = {"id": "wrap-loc-1", "text": "wrapper location record", "type": "note"} + raw_record = {"id": "raw-loc-1", "text": "raw location record", "type": "note"} + + # No --scope, no --visibility on either surface: exercise the true default + # path (wrapper injects its resolved scope + public; raw CLI uses its own + # argparse defaults). Both are public, so both route to /.eidetic/memory. + wrapper_result = _run( + ["bash", str(REMEMBER_WRAPPER), json.dumps(wrapper_record), "--json"], + cwd=repo, + env=env, + ) + assert wrapper_result.returncode == 0, wrapper_result.stderr + + raw_result = _run( + _raw_eidetic("remember", json.dumps(raw_record), "--json"), + cwd=repo, + env=env, + ) + assert raw_result.returncode == 0, raw_result.stderr + + # LOCATION (a filesystem-location claim, so a thin file check): both landed + # under /.eidetic/memory and NOT under the redirected HOME store. + repo_store = repo / ".eidetic" / "memory" + home_store = tmp_path / "home" / ".eidetic" / "memory" + assert list(repo_store.glob("*.jsonl")), "expected records in the repo-local store" + if home_store.exists(): + assert not list( + home_store.glob("*.jsonl") + ), "a public write leaked into the HOME store instead of the repo store" + + # VISIBILITY via the in-process API (not file-scraping). + records = {r.id: r for r in get_backend("files").all()} + assert "wrap-loc-1" in records, "wrapper record did not resolve to the repo store" + assert "raw-loc-1" in records, "raw CLI record did not resolve to the repo store" + assert records["wrap-loc-1"].scope.visibility == "public" + assert records["raw-loc-1"].scope.visibility == "public"