Skip to content

fix(memory): re-project MEMORY.md on every memory write, not just the CLI's - #436

Closed
sulthannauval wants to merge 1 commit into
mainfrom
fix/082-refresh-projection-on-every-memory-write
Closed

fix(memory): re-project MEMORY.md on every memory write, not just the CLI's#436
sulthannauval wants to merge 1 commit into
mainfrom
fix/082-refresh-projection-on-every-memory-write

Conversation

@sulthannauval

Copy link
Copy Markdown
Member

Summary

  • Problem: MEMORY.md is injected into every system prompt (agent/prompt.rs), and on the sqlite/lucid backends it is a projection of the core rows rather than the store itself. refresh_projection lived privately in memory/cli.rs and was called from three places, all in that file. A grep across src/tools/, src/tui/, src/gateway/ for project_core_memories|refresh_projection returned nothing.
  • Why it matters: A core memory deleted through the agent's own memory_forget, the TUI, or the HTTP API left the authoritative store but stayed in the file that reaches the model. The store side was the mirror image: a core memory written mid-session was not in the injected file at all. The projection is otherwise rebuilt only at backend construction — for the gateway and the TUI, both long-lived, that is the rest of the process lifetime, and a new session started inside that process reads the stale file.
  • What changed: refresh_projection moved to memory/snapshot.rs as a shared pub fn, gated on memory.name() instead of on config, and is now called after every successful mutation on all seven write surfaces.
  • What did NOT change (scope boundary): No write-through inside Memory::forget/store — that would put filesystem work behind a trait every test mock implements, and MarkdownMemory would recurse into its own file. No bidirectional sync between MEMORY.md and brain.db (explicitly rejected in snapshot.rs and that reasoning still holds). No mid-session prompt rebuild; the prompt is built once per session by design, and this only makes the file correct so the next session is.

Evidence that produced this, from a probe against the real tool:

the prompt-injected file still holds the forgotten entry:
<!-- rantaiclaw:memory:begin -->
- rotation_note: staging credentials rotate weekly
<!-- rantaiclaw:memory:end -->

Three controls in that probe passed: the projection did write the entry, the tool reported success, and the store no longer held it. Only the projection was stale.

Label Snapshot (required)

  • Risk label (risk: low|medium|high): risk: medium
  • Size label: auto-managed
  • Scope labels: memory, tool, gateway, agent
  • Module labels: tool: memory_store, tool: memory_forget
  • Contributor tier label: auto-managed
  • If any auto-label is incorrect, note requested correction: —

Change Metadata

  • Change type: bug
  • Primary scope: memory

Linked Issue

Validation Evidence (required)

cargo fmt --all -- --check                                              # clean
cargo +1.92.0 clippy --locked --all-targets -- -D clippy::correctness    # clean
BASE_SHA=<main> ./scripts/ci/rust_strict_delta_gate.sh                  # no blocking issues on changed lines
cargo test --lib -- memory:: memory_forget memory_store api_v1 \
    tui::commands::memory agent::agent                                  # 371 passed, 0 failed

Clippy is run under 1.92.0 because that is what .github/workflows/ci-run.yml pins; the local default toolchain is 1.97 and reports a large pre-existing diagnostic set that CI does not gate on.

  • Evidence provided: ten new tests. Eight were confirmed to fail against the previous behaviour by stubbing refresh_projection to a no-op and re-running — one per call site plus the shared helper:

    • tools::memory_forget::forget_reprojects_memory_md
    • tools::memory_forget::forget_leaves_the_other_projected_entries_alone
    • tools::memory_store::store_reprojects_memory_md
    • tools::memory_store::store_with_replaces_drops_the_superseded_entry_from_the_projection
    • gateway::api_v1::memory_delete_reprojects_memory_md
    • gateway::api_v1::memory_create_reprojects_memory_md
    • tui::commands::memory::memory_add_and_remove_reproject_memory_md
    • memory::snapshot::refresh_projects_on_sqlite

    The remaining three (refresh_is_a_no_op_on_markdown, refresh_is_a_no_op_when_memory_is_disabled, a_blocked_store_does_not_touch_the_projection) assert the gate holds and pass either way by design — they guard against this change over-reaching, not against the bug returning.

  • If any command is intentionally skipped: full-workspace cargo test was not run (disk-bound on this machine, ~27G of target); the six affected modules were run instead and are named above.

Security Impact (required)

  • New permissions/capabilities? No
  • New external network calls? No
  • Secrets/tokens handling changed? No
  • File system access scope changed? No — the projection already wrote <workspace>/MEMORY.md; this makes more call paths reach the same writer with the same path. The backend gate keeps markdown, postgres and none untouched.

Privacy and Data Hygiene (required)

  • Data-hygiene status: pass
  • Redaction/anonymization notes: none needed. Test fixtures use project-neutral values (rotation_note, user_lang, prefers Bahasa Indonesia).
  • Neutral wording confirmation: confirmed — no identity-like data in tests, comments or commit message.

Compatibility / Migration

  • Backward compatible? Yes
  • Config/env changes? No
  • Migration needed? No — an existing workspace's stale MEMORY.md block is corrected on the next write, and was already corrected on the next backend construction.

Human Verification (required)

  • Verified scenarios: forget-then-read MEMORY.md via the tool, the TUI command dispatcher and the HTTP handler; store-then-read via all three; replaces performing two mutations reflected in one rewrite; a ReadOnly-refused store leaving the file byte-identical.
  • Edge cases checked: markdown backend must be a no-op (it owns the file); none backend must not create the file; a delete must not take the surviving projected entries with it.
  • What was not verified: no live run against a real long-lived gateway or TUI process — the staleness window is asserted at unit level only. lucid is gated in alongside sqlite by name but was not exercised (its forget removes the local copy only, which is pre-existing and documented behaviour).

Side Effects / Blast Radius (required)

  • Affected subsystems: memory/snapshot, memory/cli, both memory tools, the tool factory, the agent's pre-compaction flush registry, the TUI /memory command, the gateway memory API.
  • Potential unintended effects: every successful memory write now performs one extra sqlite read plus one file write. Bounded by PROJECTION_MAX_CHARS, and it is what the CLI path already did on every memory add.
  • Guardrails: the projection is best-effort — a failure logs memory projection skipped: … via tracing::warn! and never fails the write that already succeeded.

Agent Collaboration Notes (recommended)

  • Agent tools used: Claude Code.
  • Workflow/plan summary: plans/082-forget-leaves-projected-memory-in-the-prompt.md. The plan left the helper's signature as an open question between "pass &Config" and "gate on memory.name()"; resolved to the latter after confirming each classify_memory_backend branch constructs a backend whose name() matches, making the two decisions equivalent while asking callers for only a path.
  • Verification focus: every regression test was mutation-checked — the fix was reverted and the tests re-run to confirm they actually fail. Results above.
  • Confirmation: naming + architecture boundaries followed (AGENTS.md + CONTRIBUTING.md) — yes.

Rollback Plan (required)

  • Fast rollback: git revert 7e7a73f. Single commit, no schema or config surface.
  • Feature flags: none.
  • Observable failure symptoms: MEMORY.md gaining a duplicated or malformed rantaiclaw:memory block, or memory projection skipped: … warnings appearing at a rate that tracks memory writes.

Risks and Mitigations

  • Risk: memory_flush_tools (agent/agent.rs) was not in the plan's list of call sites — it was found only because the compiler rejected the signature change. Another constructor reached by a path the compiler cannot see would have been missed the same way.

    • Mitigation: MemoryStoreTool::new/MemoryForgetTool::new are the only constructors, both take workspace_dir positionally now, and a grep -rn 'Memory\(Store\|Forget\)Tool::new' shows only tools/mod.rs and agent/agent.rs outside the tools' own tests.
  • Risk: the seven call sites each call the helper explicitly, so a write path added later can forget to.

    • Mitigation: noted rather than solved — the alternative (write-through inside the trait) is rejected above for concrete reasons. The helper is one shared function, so the fix for a future omission is one line at the new call site.
  • Risk: state_with_real_memory in the gateway tests now sets config.workspace_dir to the same TempDir as the store. That is a shared helper, so it changes the environment of every test using it.

    • Mitigation: the previous value pointed somewhere the store did not live, which is why the projection landed where the test could not see it. All 43 api_v1 tests pass with the change.

🤖 Generated with Claude Code

… CLI's

`MEMORY.md` is injected into every system prompt unconditionally
(`agent/prompt.rs`), and on the sqlite and lucid backends it is a *projection*
of the `core` rows rather than the store itself. Nothing re-projected on its own:
`refresh_projection` lived privately in `memory/cli.rs` and was called from three
places, all in that file. A grep across `src/tools/`, `src/tui/` and
`src/gateway/` for `project_core_memories|refresh_projection` returned nothing.

So a core memory deleted through the agent's own tool, the TUI, or the HTTP API
was removed from the authoritative store and stayed in the file that reaches the
model:

    the prompt-injected file still holds the forgotten entry:
    <!-- rantaiclaw:memory:begin -->
    - rotation_note: staging credentials rotate weekly
    <!-- rantaiclaw:memory:end -->

The store side was wrong in the mirror image: a `core` memory written mid-session
was not in the injected file at all. The comment at `memory/mod.rs` claiming "a
memory stored mid-session lands in the file now" was only ever true on the CLI
path.

The projection is otherwise rebuilt only at backend construction. For
`rantaiclaw run` that is the next process; for the gateway and the TUI — both
long-lived — it is the rest of the process lifetime, and a new session started
inside that process reads the stale file.

- move `refresh_projection` into `memory/snapshot.rs` as a shared `pub fn`
- gate it on `memory.name()` rather than on config. Same decision by
  construction, and it asks the caller for a workspace path instead of a
  `Config` — which the tools do not hold. `MarkdownMemory` still skips it (that
  backend owns `MEMORY.md` directly), as do `postgres` and `none`
- call it after every successful mutation on all six surfaces: `memory_forget`,
  `memory_store`, the TUI's `/memory add` and `/memory remove`, and the API's
  `memory_create` and `memory_delete`
- thread `workspace_dir` into the two memory tools. `all_tools_with_runtime`
  already had it; `memory_flush_tools` (the pre-compaction flush, which writes
  memory through the same tools) takes it from the agent

Not done, deliberately: no write-through inside `Memory::forget`/`store`. That
would put filesystem work behind a trait every test mock implements, and
`MarkdownMemory` would recurse into its own file.

Ten tests added. Eight were confirmed to fail against the previous behaviour by
stubbing `refresh_projection` to a no-op and re-running — one per call site plus
the shared helper. The other two assert the gate holds (markdown and `none` must
not be projected) and pass either way by design, as does
`a_blocked_store_does_not_touch_the_projection`.

`state_with_real_memory` in the gateway tests now points `config.workspace_dir`
at the same TempDir as the store; without that the projection lands where the
test cannot see it.

Verified: 371 tests pass across `memory::`, both memory tools, `api_v1`,
`tui::commands::memory` and `agent::agent`; `cargo fmt` clean;
`cargo +1.92.0 clippy --locked --all-targets -- -D clippy::correctness` clean.
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

PR intake checks found warnings (non-blocking)

Fast safe checks found advisory issues. CI lint/test/build gates still enforce merge quality.

  • Incomplete required PR template fields: summary problem, summary why it matters, summary what changed, validation commands, security risk/mitigation, privacy status, rollback plan

Action items:

  1. Complete required PR template sections/fields.
  2. Remove tabs, trailing whitespace, and merge conflict markers from added lines.
  3. Re-run local checks before pushing:
    • ./scripts/ci/rust_quality_gate.sh
    • ./scripts/ci/rust_strict_delta_gate.sh
    • ./scripts/ci/docs_quality_gate.sh

Run logs: https://github.com/RantAI-dev/RantAIClaw/actions/runs/31258433465

Detected blocking line issues (sample):

  • none

Detected advisory line issues (sample):

  • none

@github-actions github-actions Bot added agent Auto scope: src/agent/** changed. gateway Auto scope: src/gateway/** changed. memory Auto scope: src/memory/** changed. tool Auto scope: src/tools/** changed. size: L Auto size: 501-1000 non-doc changed lines. risk: high Auto risk: security/runtime/gateway/tools/workflows. distinguished contributor Contributor with 50+ merged PRs. gateway: api_v1 Auto module: gateway/api_v1 changed. agent: agent Auto module: agent/agent changed. and removed agent Auto scope: src/agent/** changed. gateway Auto scope: src/gateway/** changed. labels Aug 8, 2026
@sulthannauval
sulthannauval deleted the fix/082-refresh-projection-on-every-memory-write branch August 8, 2026 13:48
@sulthannauval

Copy link
Copy Markdown
Member Author

Superseded by #440 — identical commit. Closed automatically when the branch was renamed to fix/memory-projection-refresh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent: agent Auto module: agent/agent changed. distinguished contributor Contributor with 50+ merged PRs. gateway: api_v1 Auto module: gateway/api_v1 changed. memory Auto scope: src/memory/** changed. risk: high Auto risk: security/runtime/gateway/tools/workflows. size: L Auto size: 501-1000 non-doc changed lines. tool Auto scope: src/tools/** changed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant