Repository navigation
feat(memory): episodic memory — cross-session summaries + recall - #5974
tmartin2113 wants to merge 11 commits into
Conversation
… search) + terse digest/cap/open-thread refinements
…read weighting Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…session file + recent.md
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
🔎 IronLoop Review StatusHead:
Configuration errorMessage: Unable to load trusted agent config from .ironloop/agents.yaml. Available commands
Run metadataOrigin: |
github-actions
Bot
added
scope: agent
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds episodic memory for durable conversation summaries, a capped recent digest, prompt-time recall, idle-prune persistence, and startup/heartbeat backfill sweeps. Documentation and tests cover storage formats, parsing, capping, idempotency, prompt injection, and recovery behavior. ChangesEpisodic memory
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Suggested labels: Sequence Diagram(s)sequenceDiagram
participant Agent
participant SessionManager
participant SessionMemory
participant Workspace
participant HeartbeatRunner
Agent->>SessionManager: register SessionMemory
SessionManager->>SessionMemory: summarize_and_store stale conversation
SessionMemory->>Workspace: write session markdown and recent.md
Workspace->>Agent: inject recent digest into new prompt
HeartbeatRunner->>SessionMemory: sweep ended conversations
SessionMemory->>Workspace: backfill missing summaries
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/agent/session_manager.rs (1)
355-403: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winIdle-prune timestamps summaries with
Utc::now()(prune time) while the backstop sweep uses the conversation'slast_activity— this breaks the idempotency guaranteesummarize_and_storerelies on.
sessions_path's file stem is"{date}-{conversation_id}". If a stale session is pruned on a different calendar day than its last actual activity (e.g. pruned shortly after midnight for a conversation that ended just before), the idle-prune path and the backstop sweep will compute two different stems/paths for the sameconversation_id. Thefile_existscheck insummarize_and_storetherefore won't find the "other" path's file, and both paths will independently summarize and write — producing a duplicate session file and a duplicaterecent.mddigest entry for the same conversation. This is a deterministic bug (not just a race), on top of the fact that concurrently-spawned tasks here (onetokio::spawnper stale conversation) also race on the sharedrecent.mdupdate (see the related comment onsession_memory.rs::summarize_and_store).
sess.last_active_atis already read a few lines above in this same locked block and is the correct source of truth to thread through instead ofchrono::Utc::now().🐛 Proposed fix — carry the session's actual last-active timestamp
-type StaleConversation = (String, String, Vec<(String, Option<String>)>); +type StaleConversation = (String, String, Vec<(String, Option<String>)>, chrono::DateTime<chrono::Utc>); ... - stale_thread_turns.push((thread.id.to_string(), channel, turns)); + stale_thread_turns.push((thread.id.to_string(), channel, turns, sess.last_active_at)); ... - for (conversation_id, channel, turns) in stale_thread_turns { + for (conversation_id, channel, turns, ts) in stale_thread_turns { let memory = Arc::clone(memory); - let ts = chrono::Utc::now(); tokio::spawn(async move { memory .summarize_and_store(&conversation_id, &channel, ts, &turns) .await; }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agent/session_manager.rs` around lines 355 - 403, Use each stale session’s actual last-activity timestamp when scheduling episodic summaries instead of assigning chrono::Utc::now() at prune time. Extend StaleConversation and the stale_thread_turns collection to carry sess.last_active_at (or the corresponding per-conversation timestamp), then pass that timestamp to summarize_and_store in the spawned task. Also avoid spawning concurrent summaries that race on recent.md by processing the stale conversations sequentially or otherwise serializing the shared update.
🧹 Nitpick comments (4)
docs/superpowers/plans/2026-07-09-episodic-memory.md (2)
27-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFix the markdownlint violations.
Add a language identifier to the fenced block at Line 27 and remove the spaces inside the code span at Line 204.
Also applies to: 204-204
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/plans/2026-07-09-episodic-memory.md` at line 27, Fix the markdownlint violations in the plan: add an appropriate language identifier to the fenced code block around line 27, and remove the extra spaces inside the inline code span around line 204.Source: Linters/SAST tools
668-674: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the agent’s
cheap_llm()accessor.The plan reaches through
self.deps.cheap_llmdirectly. Useagent.cheap_llm()so provider selection and fallback behavior remain centralized.As per coding guidelines, code under
src/agent/must useagent.cheap_llm()rather than accessingdeps.cheap_llmdirectly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/plans/2026-07-09-episodic-memory.md` around lines 668 - 674, Update the `Agent::new` session-memory initialization to obtain the model through the agent’s `cheap_llm()` accessor instead of reading `self.deps.cheap_llm` directly, while preserving the existing fallback behavior and `SessionMemory` setup.Source: Coding guidelines
docs/superpowers/specs/2026-07-09-episodic-memory-design.md (1)
83-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a language tag to the diagram fence.
Use
```textto keep markdownlint clean.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/specs/2026-07-09-episodic-memory-design.md` at line 83, Update the diagram code fence in the episodic memory design document to specify the text language by changing the opening fence to ```text, while preserving the diagram content and closing fence.Source: Linters/SAST tools
src/agent/session_memory.rs (1)
229-253: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winNo cap on conversation size sent to the summarizer LLM call.
turnsis serialized intoconvounconditionally, regardless of conversation length. A very long-running thread (many turns before it goes idle) could exceed the model's context window, causing thecomplete()call to fail — silently swallowed by the fail-softErrbranch insummarize_and_store, so the conversation would simply never get summarized. Since the doc comment says this "mirrorsContextCompactor::generate_summary", worth confirming whether that reference implementation applies any truncation/windowing that should be mirrored here too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agent/session_memory.rs` around lines 229 - 253, The summarize method sends the entire turns history to the LLM without limiting context size. Mirror the truncation or windowing behavior used by ContextCompactor::generate_summary when building convo, preserving the most relevant recent turns within a safe model-context budget before calling self.complete. Verify summarize_and_store continues handling the bounded result correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/plans/2026-07-09-episodic-memory.md`:
- Around line 715-717: Update sweep to use the success/created boolean returned
by summarize_and_store, incrementing n only when the operation returns true;
ensure skipped trivial conversations, failures, and duplicates return false and
are not counted.
- Around line 412-417: Bound the conversation payload constructed in the
turn-building loop by reusing existing compaction/truncation logic where
available, or by enforcing maximum turn and byte limits before appending to
`convo`; stop or truncate once either limit is reached so the summarizer request
stays within provider and cost constraints.
- Around line 695-696: The SessionMemory::sweep design must not use updated_at
alone to identify completed conversations. Change the conversation enumeration
to require an explicit ended/idle state, or exclude conversations currently
marked active, before calling summarize_and_store; ensure idle-prune can still
create the final summary for active sessions.
- Around line 530-532: Make the idempotency guard in the file-writing flow
atomic: replace the separate self.file_exists(&path).await check and subsequent
write with an atomic create-if-absent operation, or protect the check-and-write
sequence with a per-conversation lock/transaction. Ensure concurrent session-end
and sweep tasks cannot both summarize or overwrite recent.md.
- Around line 662-664: Replace the untracked tokio::spawn in summarize_and_store
scheduling with a bounded background worker mechanism using a semaphore or
queue, track in-flight conversation IDs to prevent duplicate heartbeat/prune
work, and support cancellation and graceful shutdown so each conversation
produces at most one active LLM request.
- Around line 353-360: Update RawSummary deserialization and the persistence
path to enforce hard bounds on title, gist, every string in decisions,
open_threads, and user_notes, plus the total serialized summary size before
writing session files or recent-prompt content. Truncate or reject oversized
values and lists using shared constants, and ensure limits are applied before
any accumulation or serialization.
- Around line 614-623: The episodic recall path in the non-group-chat branch
injects recent.md without safety validation, delimiting, or explicit read-error
handling. Before appending in the recent-memory block, scan the content using
the applicable memory/injection safety path and only include approved content
within clearly labeled untrusted-memory delimiters; treat NotFound as absence
but explicitly handle or log other read failures.
- Around line 556-562: The workspace helpers file_exists and read_or_empty must
distinguish missing files from permission, corruption, and I/O errors. Handle
workspace.read errors explicitly: return false only for NotFound, log other
file_exists failures, and have read_or_empty preserve the existing
digest/content on non-NotFound failures instead of returning an empty string;
avoid unwrap_or_default and equivalent silent error handling.
- Around line 149-161: Update the serialization logic containing the
conversation Markdown format! block to build frontmatter from a metadata struct
or map and serialize it with a YAML serializer, rather than interpolating
self.conversation_id, self.channel, self.timestamp, and self.title directly.
Explicitly delimit or escape the Markdown body fields, including self.gist and
bullet sections, so embedded newlines, headings, colons, or --- cannot corrupt
frontmatter or document structure.
- Around line 545-552: Update the session persistence flow around the
session-file write and recent digest update so a failed RECENT_PATH write
remains retryable rather than being treated as complete. Add an atomic or
serialized read-modify-write mechanism for build_recent updates, and ensure
retries can repair an existing stale digest instead of being skipped by
idempotency checks; preserve clear warning/error handling for failures.
- Around line 654-665: Add a caller-level regression test for the idle-prune
loop’s session-memory wiring, replacing the “no new unit test” or
skip-regression guidance. Mock or spy on summarize_and_store to capture and
assert the computed conversation_id, channel, timestamp, and mapped turn pairs
for a stale thread with at least one turn; ensure the test exercises the wrapper
around OnSessionEnd and spawn behavior rather than only testing
summarize_and_store directly.
- Around line 134-137: Update `Session::file_stem` so `conversation_id` cannot
introduce path separators or traversal, using validation or a safe encoding, and
make the returned filename stable solely from the conversation ID rather than
the date. Preserve the formatted date as session metadata instead of including
it in the idempotency key.
- Around line 742-747: Update the “Step 1: Full lint + test gate” command so
Clippy failures cannot be masked by the grep pipeline. Add `set -o pipefail`
before the pipeline and retain the intended “clippy clean” fallback, or run
`cargo clippy` separately while explicitly preserving and checking its exit
status.
In `@src/agent/heartbeat.rs`:
- Around line 534-536: Move the `// arch-exempt:` justification comment in
`spawn_heartbeat` to immediately precede the
`#[allow(clippy::too_many_arguments)]` attribute, leaving the attribute directly
above the function signature.
In `@src/agent/session_manager.rs`:
- Around line 63-69: Add a public getter beside
SessionManager::set_session_memory that returns the already-wired
Arc<SessionMemory>, preserving the first instance rather than creating another.
Update agent_loop.rs::run to retrieve and reuse this instance for heartbeat
wiring, and handle the not-yet-initialized case consistently with the existing
OnceLock behavior.
In `@src/agent/session_memory.rs`:
- Around line 82-99: Update split_entries to discard all content before the
first "### " entry marker, including blank lines following "# Recent
conversations"; only append lines to cur after the first entry has started.
Preserve the existing header skip and entry-boundary behavior while ensuring no
leading newline is included in the first parsed entry.
- Around line 294-332: The RECENT_PATH read/build/write sequence in
summarize_and_store races with prune_stale_sessions and startup sweep across
separate SessionMemory instances. Serialize all recent.md updates with a
process-wide/shared lock, or replace the sequence with an atomic update
mechanism, and ensure every code path that modifies RECENT_PATH uses the same
coordination.
In `@src/workspace/mod.rs`:
- Around line 1810-1821: Add RECENT_PATH to SYSTEM_PROMPT_FILES so writes to
memory/recent.md are validated by reject_if_injected before injection. Update
the episodic recall logic in the system-prompt construction to read using
RECENT_PATH instead of the hardcoded "memory/recent.md" string.
---
Outside diff comments:
In `@src/agent/session_manager.rs`:
- Around line 355-403: Use each stale session’s actual last-activity timestamp
when scheduling episodic summaries instead of assigning chrono::Utc::now() at
prune time. Extend StaleConversation and the stale_thread_turns collection to
carry sess.last_active_at (or the corresponding per-conversation timestamp),
then pass that timestamp to summarize_and_store in the spawned task. Also avoid
spawning concurrent summaries that race on recent.md by processing the stale
conversations sequentially or otherwise serializing the shared update.
---
Nitpick comments:
In `@docs/superpowers/plans/2026-07-09-episodic-memory.md`:
- Line 27: Fix the markdownlint violations in the plan: add an appropriate
language identifier to the fenced code block around line 27, and remove the
extra spaces inside the inline code span around line 204.
- Around line 668-674: Update the `Agent::new` session-memory initialization to
obtain the model through the agent’s `cheap_llm()` accessor instead of reading
`self.deps.cheap_llm` directly, while preserving the existing fallback behavior
and `SessionMemory` setup.
In `@docs/superpowers/specs/2026-07-09-episodic-memory-design.md`:
- Line 83: Update the diagram code fence in the episodic memory design document
to specify the text language by changing the opening fence to ```text, while
preserving the diagram content and closing fence.
In `@src/agent/session_memory.rs`:
- Around line 229-253: The summarize method sends the entire turns history to
the LLM without limiting context size. Mirror the truncation or windowing
behavior used by ContextCompactor::generate_summary when building convo,
preserving the most relevant recent turns within a safe model-context budget
before calling self.complete. Verify summarize_and_store continues handling the
bounded result correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b546db4c-f007-4085-8ce7-f8c2ddf458b4
📒 Files selected for processing (9)
docs/superpowers/plans/2026-07-09-episodic-memory.mddocs/superpowers/specs/2026-07-09-episodic-memory-design.mdsrc/agent/agent_loop.rssrc/agent/heartbeat.rssrc/agent/mod.rssrc/agent/session_manager.rssrc/agent/session_memory.rssrc/workspace/README.mdsrc/workspace/mod.rs
| /// `YYYY-MM-DD-<conversation_id>` — the per-session file stem. | ||
| pub fn file_stem(&self) -> String { | ||
| format!("{}-{}", self.timestamp.format("%Y-%m-%d"), self.conversation_id) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Make the session filename safe and stable per conversation ID.
conversation_id is inserted directly into a path, so separators or .. can escape memory/sessions/. Also, the date is part of the idempotency path; the same conversation summarized across midnight can create multiple files. Validate/encode the identifier and use a stable conversation-keyed path, keeping the date as metadata.
Suggested direction
- format!("{}-{}", self.timestamp.format("%Y-%m-%d"), self.conversation_id)
+ format!("{}-{}", self.timestamp.format("%Y-%m-%d"), self.conversation_id.safe_file_component())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-07-09-episodic-memory.md` around lines 134 - 137,
Update `Session::file_stem` so `conversation_id` cannot introduce path
separators or traversal, using validation or a safe encoding, and make the
returned filename stable solely from the conversation ID rather than the date.
Preserve the formatted date as session metadata instead of including it in the
idempotency key.
Source: Coding guidelines
| format!( | ||
| "---\nconversation_id: {}\nchannel: {}\ntimestamp: {}\ntitle: {}\n---\n\n\ | ||
| # {}\n\n{}\n\n## Decisions\n{}\n## Open threads\n{}\n## User notes\n{}", | ||
| self.conversation_id, | ||
| self.channel, | ||
| self.timestamp.to_rfc3339(), | ||
| self.title, | ||
| self.title, | ||
| self.gist, | ||
| Self::bullets(&self.decisions), | ||
| Self::bullets(&self.open_threads), | ||
| Self::bullets(&self.user_notes), | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Serialize frontmatter instead of interpolating unescaped values.
Titles, channels, and model-generated text can contain newlines, :, ---, or markdown headings. Direct format! interpolation can corrupt YAML frontmatter and the searchable document format. Use a YAML serializer for metadata and explicitly delimit/escape the markdown body.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-07-09-episodic-memory.md` around lines 149 - 161,
Update the serialization logic containing the conversation Markdown format!
block to build frontmatter from a metadata struct or map and serialize it with a
YAML serializer, rather than interpolating self.conversation_id, self.channel,
self.timestamp, and self.title directly. Explicitly delimit or escape the
Markdown body fields, including self.gist and bullet sections, so embedded
newlines, headings, colons, or --- cannot corrupt frontmatter or document
structure.
| #[derive(Deserialize, Default)] | ||
| struct RawSummary { | ||
| #[serde(default)] title: String, | ||
| #[serde(default)] gist: String, | ||
| #[serde(default)] decisions: Vec<String>, | ||
| #[serde(default)] open_threads: Vec<String>, | ||
| #[serde(default)] user_notes: Vec<String>, | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Bound parsed summary fields before persistence.
#[serde(default)] handles missing fields but does not limit list length or string size. A large model response can create oversized session files and recent-prompt content. Enforce limits for title, gist, each list, and total serialized size before writing.
As per coding guidelines, user-controlled inputs and accumulators require hard resource limits.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-07-09-episodic-memory.md` around lines 353 - 360,
Update RawSummary deserialization and the persistence path to enforce hard
bounds on title, gist, every string in decisions, open_threads, and user_notes,
plus the total serialized summary size before writing session files or
recent-prompt content. Truncate or reject oversized values and lists using
shared constants, and ensure limits are applied before any accumulation or
serialization.
Source: Coding guidelines
| let mut convo = String::new(); | ||
| for (u, a) in turns { | ||
| convo.push_str(&format!("User: {u}\n")); | ||
| if let Some(a) = a { | ||
| convo.push_str(&format!("Assistant: {a}\n")); | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Bound the conversation payload sent to the summarizer.
This loop concatenates every turn into one unbounded String. Long conversations can exceed provider context limits or create expensive background requests. Reuse compaction/truncation logic or enforce a maximum number of turns and bytes before building the request.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-07-09-episodic-memory.md` around lines 412 - 417,
Bound the conversation payload constructed in the turn-building loop by reusing
existing compaction/truncation logic where available, or by enforcing maximum
turn and byte limits before appending to `convo`; stop or truncate once either
limit is reached so the summarizer request stays within provider and cost
constraints.
Source: Coding guidelines
| // idempotency: if a file already exists, skip (per confirm-first read semantics) | ||
| if self.file_exists(&path).await { | ||
| return; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make idempotency atomic.
The existence check is a read-then-write race: concurrent session-end and sweep tasks can both observe absence, summarize twice, and overwrite/duplicate recent.md. Use an atomic create-if-absent operation or a per-conversation lock/transaction.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-07-09-episodic-memory.md` around lines 530 - 532,
Make the idempotency guard in the file-writing flow atomic: replace the separate
self.file_exists(&path).await check and subsequent write with an atomic
create-if-absent operation, or protect the check-and-write sequence with a
per-conversation lock/transaction. Ensure concurrent session-end and sweep tasks
cannot both summarize or overwrite recent.md.
| #[allow(clippy::too_many_arguments)] | ||
| // arch-exempt: too_many_args, heartbeat spawn threads optional deps individually; a bundle struct is a future cleanup, plan 2026-07-09-episodic-memory | ||
| pub fn spawn_heartbeat( |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
arch-exempt comment is placed after #[allow(clippy::too_many_arguments)] instead of directly above it.
Per the retrieved learning/coding guideline, the exemption comment must sit immediately above the #[allow(...)] attribute. Here it's between the attribute and the function signature.
✏️ Fix ordering
-#[allow(clippy::too_many_arguments)]
-// arch-exempt: too_many_args, heartbeat spawn threads optional deps individually; a bundle struct is a future cleanup, plan 2026-07-09-episodic-memory
+// arch-exempt: too_many_args, heartbeat spawn threads optional deps individually; a bundle struct is a future cleanup, plan 2026-07-09-episodic-memory
+#[allow(clippy::too_many_arguments)]
pub fn spawn_heartbeat(Based on learnings, an "architecture exemption" comment must be placed immediately above the #[allow(...)] attribute explaining the justification. As per coding guidelines, "place // arch-exempt: ... directly above it."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[allow(clippy::too_many_arguments)] | |
| // arch-exempt: too_many_args, heartbeat spawn threads optional deps individually; a bundle struct is a future cleanup, plan 2026-07-09-episodic-memory | |
| pub fn spawn_heartbeat( | |
| // arch-exempt: too_many_args, heartbeat spawn threads optional deps individually; a bundle struct is a future cleanup, plan 2026-07-09-episodic-memory | |
| #[allow(clippy::too_many_arguments)] | |
| pub fn spawn_heartbeat( |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agent/heartbeat.rs` around lines 534 - 536, Move the `// arch-exempt:`
justification comment in `spawn_heartbeat` to immediately precede the
`#[allow(clippy::too_many_arguments)]` attribute, leaving the attribute directly
above the function signature.
Sources: Coding guidelines, Learnings
| /// Wire the episodic-memory coordinator. Callable on a shared `Arc` | ||
| /// (takes `&self`); the first call wins and later calls are ignored. | ||
| pub fn set_session_memory(&self, memory: Arc<SessionMemory>) { | ||
| // Ignore a redundant second wiring — the store is idempotent anyway. | ||
| let _ = self.session_memory.set(memory); | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Consider exposing a getter so callers can reuse the wired SessionMemory instance instead of constructing a second one.
Agent::new builds a SessionMemory and wires it here via set_session_memory, but there's no way for agent_loop.rs::run() to retrieve it later — which is why it currently constructs a second, independent SessionMemory for heartbeat wiring (see the corresponding comment on agent_loop.rs). A simple accessor avoids that duplication and is a prerequisite for making any future per-instance lock on the recent.md critical section actually effective across both call sites.
♻️ Proposed getter
+ /// Get the wired episodic-memory coordinator, if any.
+ pub fn session_memory(&self) -> Option<Arc<SessionMemory>> {
+ self.session_memory.get().cloned()
+ }
+
/// Get or create a session for a user.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Wire the episodic-memory coordinator. Callable on a shared `Arc` | |
| /// (takes `&self`); the first call wins and later calls are ignored. | |
| pub fn set_session_memory(&self, memory: Arc<SessionMemory>) { | |
| // Ignore a redundant second wiring — the store is idempotent anyway. | |
| let _ = self.session_memory.set(memory); | |
| } | |
| /// Wire the episodic-memory coordinator. Callable on a shared `Arc` | |
| /// (takes `&self`); the first call wins and later calls are ignored. | |
| pub fn set_session_memory(&self, memory: Arc<SessionMemory>) { | |
| // Ignore a redundant second wiring — the store is idempotent anyway. | |
| let _ = self.session_memory.set(memory); | |
| } | |
| /// Get the wired episodic-memory coordinator, if any. | |
| pub fn session_memory(&self) -> Option<Arc<SessionMemory>> { | |
| self.session_memory.get().cloned() | |
| } | |
| /// Get or create a session for a user. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agent/session_manager.rs` around lines 63 - 69, Add a public getter
beside SessionManager::set_session_memory that returns the already-wired
Arc<SessionMemory>, preserving the first instance rather than creating another.
Update agent_loop.rs::run to retrieve and reuse this instance for heartbeat
wiring, and handle the not-yet-initialized case consistently with the existing
OnceLock behavior.
| fn split_entries(body: &str) -> Vec<String> { | ||
| let mut out = Vec::new(); | ||
| let mut cur = String::new(); | ||
| for line in body.lines() { | ||
| if line.starts_with("### ") && !cur.trim().is_empty() { | ||
| out.push(std::mem::take(&mut cur)); | ||
| } | ||
| if line.starts_with("# Recent conversations") { | ||
| continue; | ||
| } | ||
| cur.push_str(line); | ||
| cur.push('\n'); | ||
| } | ||
| if !cur.trim().is_empty() { | ||
| out.push(cur); | ||
| } | ||
| out | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
split_entries leaks the header's blank line into the first parsed entry, causing whitespace to accumulate on every round-trip.
After the "# Recent conversations" line is skipped via continue, the following blank line from RECENT_HEADER ("\n\n") falls through into cur before the first "### " line is seen. Since cur.trim().is_empty() is still true at that point, the push-on-boundary branch never fires, so the leading "\n" sticks to whatever entry currently sits right after the header. On the next build_recent call that entry is written back with e.trim_end() (which only trims the end), so the extra leading newline (and, on subsequent rounds, extra blank lines) is retained and grows with each round-trip that entry survives — silently eating into the max_chars budget and causing premature eviction of otherwise-wanted entries.
🐛 Proposed fix — don't accumulate anything before the first entry marker
fn split_entries(body: &str) -> Vec<String> {
let mut out = Vec::new();
let mut cur = String::new();
+ let mut started = false;
for line in body.lines() {
- if line.starts_with("### ") && !cur.trim().is_empty() {
- out.push(std::mem::take(&mut cur));
- }
if line.starts_with("# Recent conversations") {
continue;
}
+ if line.starts_with("### ") {
+ if started && !cur.trim().is_empty() {
+ out.push(std::mem::take(&mut cur));
+ }
+ started = true;
+ }
+ if !started {
+ continue;
+ }
cur.push_str(line);
cur.push('\n');
}
if !cur.trim().is_empty() {
out.push(cur);
}
out
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn split_entries(body: &str) -> Vec<String> { | |
| let mut out = Vec::new(); | |
| let mut cur = String::new(); | |
| for line in body.lines() { | |
| if line.starts_with("### ") && !cur.trim().is_empty() { | |
| out.push(std::mem::take(&mut cur)); | |
| } | |
| if line.starts_with("# Recent conversations") { | |
| continue; | |
| } | |
| cur.push_str(line); | |
| cur.push('\n'); | |
| } | |
| if !cur.trim().is_empty() { | |
| out.push(cur); | |
| } | |
| out | |
| } | |
| fn split_entries(body: &str) -> Vec<String> { | |
| let mut out = Vec::new(); | |
| let mut cur = String::new(); | |
| let mut started = false; | |
| for line in body.lines() { | |
| if line.starts_with("# Recent conversations") { | |
| continue; | |
| } | |
| if line.starts_with("### ") { | |
| if started && !cur.trim().is_empty() { | |
| out.push(std::mem::take(&mut cur)); | |
| } | |
| started = true; | |
| } | |
| if !started { | |
| continue; | |
| } | |
| cur.push_str(line); | |
| cur.push('\n'); | |
| } | |
| if !cur.trim().is_empty() { | |
| out.push(cur); | |
| } | |
| out | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agent/session_memory.rs` around lines 82 - 99, Update split_entries to
discard all content before the first "### " entry marker, including blank lines
following "# Recent conversations"; only append lines to cur after the first
entry has started. Preserve the existing header skip and entry-boundary behavior
while ensuring no leading newline is included in the first parsed entry.
| pub async fn summarize_and_store( | ||
| &self, | ||
| conversation_id: &str, | ||
| channel: &str, | ||
| timestamp: DateTime<Utc>, | ||
| turns: &[(String, Option<String>)], | ||
| ) { | ||
| // Skip trivial: nothing worth remembering if every user turn is empty. | ||
| if turns.iter().all(|(u, _)| u.trim().is_empty()) { | ||
| return; | ||
| } | ||
| let stem = format!("{}-{}", timestamp.format("%Y-%m-%d"), conversation_id); | ||
| let path = sessions_path(&stem); | ||
| // Idempotency: a file already existing for this conversation means we | ||
| // already summarized it — skip (guards session-end vs backstop races). | ||
| if self.file_exists(&path).await { | ||
| return; | ||
| } | ||
| let summary = match self | ||
| .summarizer | ||
| .summarize(conversation_id, channel, timestamp, turns) | ||
| .await | ||
| { | ||
| Ok(s) => s, | ||
| Err(e) => { | ||
| tracing::warn!("session summary failed for {conversation_id}: {e}"); | ||
| return; | ||
| } | ||
| }; | ||
| if let Err(e) = self.workspace.write(&path, &summary.to_markdown()).await { | ||
| tracing::warn!("write session file failed for {conversation_id}: {e}"); | ||
| return; | ||
| } | ||
| let existing = self.read_or_empty(RECENT_PATH).await; | ||
| let recent = build_recent(&summary.digest_entry(), &existing, 5, 6000); | ||
| if let Err(e) = self.workspace.write(RECENT_PATH, &recent).await { | ||
| tracing::warn!("update recent.md failed for {conversation_id}: {e}"); | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## session_memory.rs outline\n'
ast-grep outline src/agent/session_memory.rs --view expanded | sed -n '1,220p'
printf '\n## session_manager.rs outline\n'
ast-grep outline src/agent/session_manager.rs --view expanded | sed -n '1,260p'
printf '\n## agent_loop.rs outline\n'
ast-grep outline src/agent/agent_loop.rs --view expanded | sed -n '1,260p'
printf '\n## search for RECENT_PATH and recent.md writers\n'
rg -n "RECENT_PATH|recent\.md|build_recent\(" src/agent src/workspace crates -S
printf '\n## search for SessionMemory construction and sharing\n'
rg -n "SessionMemory::new|SessionMemory \{" src/agent -S
printf '\n## inspect workspace append docs / implementation references\n'
rg -n "Uses a read-modify-write pattern that is not concurrency-safe|append\(" src/workspace -SRepository: nearai/ironclaw
Length of output: 17492
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## session_memory.rs around summarize_and_store and sweep\n'
sed -n '274,360p' src/agent/session_memory.rs | cat -n
printf '\n## session_memory.rs around read_or_empty / tests\n'
sed -n '389,635p' src/agent/session_memory.rs | cat -n
printf '\n## session_manager.rs around prune_stale_sessions\n'
sed -n '321,420p' src/agent/session_manager.rs | cat -n
printf '\n## agent_loop.rs around SessionMemory construction sites\n'
sed -n '560,620p' src/agent/agent_loop.rs | cat -n
printf '\n---\n'
sed -n '1300,1355p' src/agent/agent_loop.rs | cat -n
printf '\n## workspace write / append behavior\n'
sed -n '1180,1215p' src/workspace/mod.rs | cat -nRepository: nearai/ironclaw
Length of output: 28651
Serialize memory/recent.md updates
summary_and_store does a read → build → write on RECENT_PATH, and prune_stale_sessions plus the startup sweep can run it concurrently. A per-instance mutex won’t close the gap because Agent::new creates separate SessionMemory instances for the idle-prune and heartbeat paths. Use a shared lock for this file or make the update atomic.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agent/session_memory.rs` around lines 294 - 332, The RECENT_PATH
read/build/write sequence in summarize_and_store races with prune_stale_sessions
and startup sweep across separate SessionMemory instances. Serialize all
recent.md updates with a process-wide/shared lock, or replace the sequence with
an atomic update mechanism, and ensure every code path that modifies RECENT_PATH
uses the same coordination.
| // Episodic recall: inject the recent-conversations digest (memory/recent.md). | ||
| // This is personal continuity context, so it is excluded in group chats the | ||
| // same way MEMORY.md is. Missing/empty is graceful — the section is omitted. | ||
| if !is_group_chat | ||
| && let Ok(doc) = self.read("memory/recent.md").await | ||
| { | ||
| let recent = doc.content.trim(); | ||
| if !recent.is_empty() { | ||
| parts.push(recent.to_string()); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n -B2 -A20 'fn is_system_prompt_file|SYSTEM_PROMPT_FILES' src/workspaceRepository: nearai/ironclaw
Length of output: 4599
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the write path and recent-memory constants to see whether memory/recent.md
# is covered by the same prompt-injection scan as other system-prompt files.
rg -n -A80 -B20 'reject_if_injected|is_system_prompt_file|RECENT_PATH|MEMORY' src/workspace/mod.rs src/agent/session_memory.rsRepository: nearai/ironclaw
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the exact definitions for the paths used by workspace injection and recent recall.
cat -n src/agent/session_memory.rs | sed -n '1,220p'Repository: nearai/ironclaw
Length of output: 8639
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the workspace write logic around prompt-injection scanning and the new recall injection block.
cat -n src/workspace/mod.rs | sed -n '150,240p;1680,1845p'Repository: nearai/ironclaw
Length of output: 11816
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for any other references to memory/recent.md or RECENT_PATH in the workspace.
rg -n 'memory/recent\.md|RECENT_PATH|recent\.md' srcRepository: nearai/ironclaw
Length of output: 2343
Add memory/recent.md to the prompt-injection scan. memory/recent.md is injected into the system prompt, but it is not in SYSTEM_PROMPT_FILES, so writes to it skip reject_if_injected. Any malicious content that lands in the digest will be replayed into later conversations. Reuse RECENT_PATH here instead of hardcoding the path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/workspace/mod.rs` around lines 1810 - 1821, Add RECENT_PATH to
SYSTEM_PROMPT_FILES so writes to memory/recent.md are validated by
reject_if_injected before injection. Update the episodic recall logic in the
system-prompt construction to read using RECENT_PATH instead of the hardcoded
"memory/recent.md" string.
|
Thank you for the thoughtful episodic-memory design and for focusing on cross-session continuity.\n\nWe are closing this legacy implementation because the Reborn memory lifecycle is now represented by #5327, which provides host-managed long-term/thread-local retrieval, safe prompt injection, and idempotent after-turn interaction recording.\n\nReborn references:\n- https://github.com/nearai/ironclaw/pull/5327\n- https://github.com/nearai/ironclaw/blob/main/crates/ironclaw_memory/src/service.rs\n- https://github.com/nearai/ironclaw/blob/main/crates/ironclaw_memory_native/src/service.rs\n\nDurable distilled on-run-end summaries are explicitly tracked as the deferred Phase 3 of #5327. That is the right place to continue this idea rather than porting the v1 session-prune and heartbeat implementation. We would be very happy to have you contribute to that Reborn follow-up, with this PR preserved as design context and attribution.\n\nThis is an architecture-transition closure, not a reflection on the quality or value of your contribution. |
2 participants
Episodic memory — cross-session summaries + recall
Automatic cross-session continuity: distill each conversation into a durable, searchable summary when it ends, and silently inject a terse digest of recent ones into new conversations.
Two-channel recall
memory/recent.mddigest (last N=5 / ~6000 chars, open-thread-weighted) injected into the system prompt (group-chat-excluded, graceful when absent).memory/sessions/YYYY-MM-DD-<id>.mdfiles, searchable via the existing hybrid FTS+vector RRF search.Write paths — all fail-soft, idempotent on
conversation_idSessionSummarizer(turns → structured JSON via the LLM) with a tolerant JSON parse + fallback.SessionMemory::summarize_and_store— skip-trivial, idempotent, writes the session file + updatesrecent.md.Memory never blocks a turn (log-and-continue everywhere). Cherry-picked from an integration branch; PR CI validates standalone.
🤖 Generated with Claude Code