Skip to content

Inject memory file path into inner Claude context#149

Merged
dcellison merged 3 commits intomainfrom
fix/memory-path-injection
Mar 24, 2026
Merged

Inject memory file path into inner Claude context#149
dcellison merged 3 commits intomainfrom
fix/memory-path-injection

Conversation

@dcellison
Copy link
Copy Markdown
Owner

Summary

  • Include the absolute file path in the memory injection label so the inner Claude knows where to write back
  • Update CLAUDE.md to reference the session context instead of the old hardcoded .claude/MEMORY.md path

Since PR #144 moved runtime data to DATA_DIR/memory/MEMORY.md, the inner Claude has been told to write to a path that no longer exists. The context injection provided memory contents for reading but not the file path for writing. Now the injection label includes both:

[Your persistent memory (file: /var/lib/kai/memory/MEMORY.md):]
{memory content}

Test plan

  • All 1087 tests pass, make check clean
  • Existing context injection tests unaffected (they check for content, not label text)
  • Manual: send a message, verify the context prefix contains the absolute memory path
  • Manual: ask the inner Claude to remember something, verify it writes to the correct file

Fixes #147

The context injection included memory contents but not the file path,
so the inner Claude could read memory but not write back to it. Include
the absolute path in the injection label so the inner Claude knows
where to save.

Also update CLAUDE.md to reference the session context instead of the
old hardcoded .claude/MEMORY.md path which has been wrong since PR #144
moved runtime data to DATA_DIR.

Fixes #147
@dcellison
Copy link
Copy Markdown
Owner Author

Review by Kai

Review: Fix inner Claude memory path and enable persistent memory writes

Critical: None

Warning

Empty/missing memory skips path injection — bootstrapping problem
claude.py:405-407: The path is only injected when memory is non-empty. On a fresh install (or after clearing memory), the if memory: guard skips the block entirely. The inner Claude receives no path, and the updated CLAUDE.md instruction ("path is provided in your session context") is simply false. The PR only fixes the bug for users who already have content in their memory file.

Fix: inject the path label even when memory is empty, or inject it unconditionally as a separate prefix:

parts.append(f"[Your persistent memory (file: {memory_path}):]\n{memory if memory else '(empty)'}")

Suggestion

Absolute path disclosure in prompts
claude.py:406: The resolved DATA_DIR path (e.g. /var/lib/kai/memory/MEMORY.md) will appear in every conversation's context prefix. If prompts are logged, this leaks internal directory structure to wherever logs go. Low severity since it's a server-side system path, not credentials — but worth noting.

CLAUDE.md instruction is vague on failure
workspace/.claude/CLAUDE.md:41: "injected on first message" is correct when memory exists, but gives the inner Claude no fallback guidance if the injection is absent (see warning above). Tightening this to "if no path appears in context, do not attempt memory writes" would avoid a confused Claude guessing paths.

Clean

The memory_path value comes from application config (a Path object), not user input, so there is no prompt injection risk from the path itself. The memory content appears after the label, so a malicious content string cannot spoof the path label.

Bottom line: The core idea is sound, but the bootstrapping case (empty memory on first use) is a real bug that will reproduce the original issue for new installs. Block on that fix before merging.

@dcellison dcellison changed the title Fix inner Claude memory path and enable persistent memory writes Inject memory file path into inner Claude context Mar 24, 2026
On a fresh install the memory file is empty, so the if-memory guard
skipped the block entirely and the inner Claude never received the
path. Now injects the path unconditionally: with content when
available, or a note that the file is empty/not yet created.
@dcellison
Copy link
Copy Markdown
Owner Author

Review by Kai

Review: Inject memory file path into inner Claude context

The prior review's blocking concern (bootstrapping bug) is fixed. All three cases now inject the path. The fix is correct.

Suggestion

Inconsistent message format across the three cases (claude.py:406-411):

The "has content" case uses bracket notation ending with a colon:

[Your persistent memory (file: /path/MEMORY.md):]
{content}

The empty and not-created cases switch to a different phrasing ending with a period:

[Your persistent memory file is at /path/MEMORY.md (currently empty).]
[Your persistent memory file is at /path/MEMORY.md (not yet created).]

If the inner Claude is pattern-matching on the prefix to extract the path, the inconsistency between these formats could cause it to miss the path in the empty/not-created cases. Unifying the format would make extraction more reliable:

parts.append(f"[Your persistent memory (file: {memory_path}):]\n(currently empty)")
parts.append(f"[Your persistent memory (file: {memory_path}):]\n(not yet created)")

Clean

  • memory_path comes from application config (a Path object), not user input — no prompt injection risk from the path itself.
  • Memory content appears after the label, so a malicious memory string cannot spoof the path label.
  • The CLAUDE.md instruction "injected on first message" is now accurate since all three cases inject unconditionally.

Bottom line: The blocking issue from the prior review is resolved. Only the format inconsistency is worth a look before merging.

@dcellison
Copy link
Copy Markdown
Owner Author

Review by Kai

Review: Inject memory file path into inner Claude context

The format inconsistency flagged in the prior review is resolved. All three cases now use the same bracket structure:

[Your persistent memory (file: {memory_path}):]
{content | (currently empty) | (not yet created)}

Suggestion

Unhandled read error (claude.py:405): memory_path.read_text() has no error handling. A permissions error or concurrent truncation would surface as an unhandled exception. This is pre-existing behavior (the old code had the same gap), but the new branch structure is a natural place to add a try/except OSError fallback — e.g., treat a read failure the same as "not yet created" and log a warning.

Clean

  • Bootstrapping bug (prior blocking issue): fixed — all three branches inject the path unconditionally.
  • Format consistency (prior suggestion): fixed — bracket notation with (file: ...) is uniform.
  • memory_path comes from config (Path object), not user input — no prompt injection risk from the path.
  • Memory content appears after the label — a malicious memory string cannot spoof the path label.
  • CLAUDE.md instruction "injected on first message" is now accurate.

Bottom line: Ready to merge. The read-error gap is worth a follow-up but is pre-existing and out of scope for this fix.

@dcellison dcellison merged commit 02da767 into main Mar 24, 2026
1 check passed
@dcellison dcellison deleted the fix/memory-path-injection branch March 24, 2026 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix inner Claude memory path

2 participants