fix(memory): re-project MEMORY.md on every memory write, not just the CLI's - #436
Closed
sulthannauval wants to merge 1 commit into
Closed
fix(memory): re-project MEMORY.md on every memory write, not just the CLI's#436sulthannauval wants to merge 1 commit into
sulthannauval wants to merge 1 commit into
Conversation
… 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.
PR intake checks found warnings (non-blocking)Fast safe checks found advisory issues. CI lint/test/build gates still enforce merge quality.
Action items:
Run logs: https://github.com/RantAI-dev/RantAIClaw/actions/runs/31258433465 Detected blocking line issues (sample):
Detected advisory line issues (sample):
|
sulthannauval
deleted the
fix/082-refresh-projection-on-every-memory-write
branch
August 8, 2026 13:48
Member
Author
|
Superseded by #440 — identical commit. Closed automatically when the branch was renamed to |
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
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.
Summary
MEMORY.mdis injected into every system prompt (agent/prompt.rs), and on the sqlite/lucid backends it is a projection of thecorerows rather than the store itself.refresh_projectionlived privately inmemory/cli.rsand was called from three places, all in that file. A grep acrosssrc/tools/,src/tui/,src/gateway/forproject_core_memories|refresh_projectionreturned nothing.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: acorememory 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.refresh_projectionmoved tomemory/snapshot.rsas a sharedpub fn, gated onmemory.name()instead of on config, and is now called after every successful mutation on all seven write surfaces.Memory::forget/store— that would put filesystem work behind a trait every test mock implements, andMarkdownMemorywould recurse into its own file. No bidirectional sync betweenMEMORY.mdandbrain.db(explicitly rejected insnapshot.rsand 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:
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: low|medium|high):risk: mediummemory,tool,gateway,agenttool: memory_store,tool: memory_forgetChange Metadata
bugmemoryLinked Issue
Validation Evidence (required)
Clippy is run under
1.92.0because that is what.github/workflows/ci-run.ymlpins; 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_projectionto a no-op and re-running — one per call site plus the shared helper:tools::memory_forget::forget_reprojects_memory_mdtools::memory_forget::forget_leaves_the_other_projected_entries_alonetools::memory_store::store_reprojects_memory_mdtools::memory_store::store_with_replaces_drops_the_superseded_entry_from_the_projectiongateway::api_v1::memory_delete_reprojects_memory_mdgateway::api_v1::memory_create_reprojects_memory_mdtui::commands::memory::memory_add_and_remove_reproject_memory_mdmemory::snapshot::refresh_projects_on_sqliteThe 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 testwas not run (disk-bound on this machine, ~27G of target); the six affected modules were run instead and are named above.Security Impact (required)
<workspace>/MEMORY.md; this makes more call paths reach the same writer with the same path. The backend gate keepsmarkdown,postgresandnoneuntouched.Privacy and Data Hygiene (required)
passrotation_note,user_lang,prefers Bahasa Indonesia).Compatibility / Migration
MEMORY.mdblock is corrected on the next write, and was already corrected on the next backend construction.Human Verification (required)
MEMORY.mdvia the tool, the TUI command dispatcher and the HTTP handler; store-then-read via all three;replacesperforming two mutations reflected in one rewrite; aReadOnly-refused store leaving the file byte-identical.markdownbackend must be a no-op (it owns the file);nonebackend must not create the file; a delete must not take the surviving projected entries with it.lucidis gated in alongsidesqliteby name but was not exercised (itsforgetremoves the local copy only, which is pre-existing and documented behaviour).Side Effects / Blast Radius (required)
memory/snapshot,memory/cli, both memory tools, the tool factory, the agent's pre-compaction flush registry, the TUI/memorycommand, the gateway memory API.PROJECTION_MAX_CHARS, and it is what the CLI path already did on everymemory add.memory projection skipped: …viatracing::warn!and never fails the write that already succeeded.Agent Collaboration Notes (recommended)
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 onmemory.name()"; resolved to the latter after confirming eachclassify_memory_backendbranch constructs a backend whosename()matches, making the two decisions equivalent while asking callers for only a path.AGENTS.md+CONTRIBUTING.md) — yes.Rollback Plan (required)
git revert 7e7a73f. Single commit, no schema or config surface.MEMORY.mdgaining a duplicated or malformedrantaiclaw:memoryblock, ormemory 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.MemoryStoreTool::new/MemoryForgetTool::neware the only constructors, both takeworkspace_dirpositionally now, and agrep -rn 'Memory\(Store\|Forget\)Tool::new'shows onlytools/mod.rsandagent/agent.rsoutside the tools' own tests.Risk: the seven call sites each call the helper explicitly, so a write path added later can forget to.
Risk:
state_with_real_memoryin the gateway tests now setsconfig.workspace_dirto the same TempDir as the store. That is a shared helper, so it changes the environment of every test using it.api_v1tests pass with the change.🤖 Generated with Claude Code