feat(install): disable Claude Code built-in memory during setup#155
feat(install): disable Claude Code built-in memory during setup#155
Conversation
Claude Code maintains its own flat-file memory system (~/.claude/projects/*/memory/) that competes with Ghost: both systems write memories, the file-memory injects stale/conflicting context at session start, and Claude consults it before Ghost's richer store because it appears to have already loaded an answer. Fix: ghost mcp init now sets "autoMemoryEnabled": false in ~/.claude/settings.json (step 5/7) to stop Claude Code writing to its own file-memory during sessions where Ghost is active. - settings.go: add setAutoMemoryEnabled / getAutoMemoryEnabled on settingsFile; idempotent, merges with existing settings, never clobbers other keys - init.go: add ensureAutoMemoryDisabled() as step 5; dry-run aware; rides the same atomic save as permissions/hook steps - status.go: surface autoMemoryEnabled check in ghost mcp status - README.md: update step table (6→7 steps) and document the new step - settings_test.go: 7 new tests covering absent, idempotent, override, key-preservation, and round-trip cases - init_test.go: 3 new tests covering set, idempotent, and dry-run cases Signed-off-by: wcatz <wayne@blinklabs.io> Signed-off-by: wcatz <waynecataldo@gmail.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 56 minutes and 34 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThe PR adds an explicit step to disable Claude Code's built-in flat-file memory by writing Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/mcpinit/status.go (1)
88-88:⚠️ Potential issue | 🟡 MinorStale step number in comment.
With the added autoMemoryEnabled check renumbering settings checks to
4-6, the project-redirects comment should be// 7.instead of// 6..📝 Suggested fix
- // 6. Project redirects. + // 7. Project redirects.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/mcpinit/status.go` at line 88, Update the stale comment numbering for the project-redirects step in internal/mcpinit/status.go: change the comment text "// 6. Project redirects." to "// 7. Project redirects." to reflect the inserted autoMemoryEnabled check that pushed the settings checks to 4-6; locate the comment near the settings validation block where steps are enumerated and adjust only the numeric label.internal/mcpinit/init.go (1)
27-75:⚠️ Potential issue | 🔴 CriticalCI blocker: errcheck failing on modified
fmt.Fprintlncalls.
golangci-lintis failing on the renumbered banner lines ([1/7],[2/7],[3/7]at lines 27, 34, 40, and likely the rest of the [N/7] lines that were touched). Even though these are just text-renumber edits, modifying the lines re-triggers the errcheck rule.Quickest fix is to discard the return explicitly on each touched call, e.g.:
📝 Suggested fix pattern
- fmt.Fprintln(w, "[1/7] Checking prerequisites...") + _, _ = fmt.Fprintln(w, "[1/7] Checking prerequisites...")Apply to lines 27, 34, 40, 47, 53, 68, 75 (and any other touched
fmt.Fprintln/fmt.Fprintfcalls flagged by CI).Alternatively, add a small local helper or relax the errcheck config — but the pipeline must be green before merge.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/mcpinit/init.go` around lines 27 - 75, The failing errcheck is due to unchecked returns from the fmt.Fprintln/Fprintf calls in init.go around the MCP init flow; update each touched call (the banners and the import error print near the functions checkPrereqs, registerMCP, ensurePermissions, ensureHook, ensureAutoMemoryDisabled, importMemories and the settings write block) to explicitly discard the returned values (e.g., assign to the blank identifier) so the error is considered handled; apply this to all modified fmt.Fprintln/Fprintf invocations in that snippet so CI passes.
🧹 Nitpick comments (1)
internal/mcpinit/init_test.go (1)
160-253: Good coverage of the three branches (set, idempotent, dry-run).Tests verify both output messages and in-memory state, including the important dry-run invariant that settings are not mutated.
One small gap: no test exercises the
autoMemoryEnabled: true→ would-overwrite path in dry-run mode (i.e., dry-run should still emitwould set ... falsewhen the key currently holdstrue). Optional to add.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/mcpinit/init_test.go` around lines 160 - 253, Add a new unit test that covers the dry-run branch when the current setting is true: create a temp HOME and settings.json containing {"autoMemoryEnabled":true}, call loadSettings and then ensureAutoMemoryDisabled with dryRun=true, assert the output contains "would set autoMemoryEnabled: false", and assert the in-memory setting (sf.getAutoMemoryEnabled()) remains true (i.e., dry run does not mutate state). Use the existing test patterns in TestEnsureAutoMemoryDisabled_DryRun to locate usage of loadSettings, ensureAutoMemoryDisabled, and sf.getAutoMemoryEnabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@internal/mcpinit/init.go`:
- Around line 27-75: The failing errcheck is due to unchecked returns from the
fmt.Fprintln/Fprintf calls in init.go around the MCP init flow; update each
touched call (the banners and the import error print near the functions
checkPrereqs, registerMCP, ensurePermissions, ensureHook,
ensureAutoMemoryDisabled, importMemories and the settings write block) to
explicitly discard the returned values (e.g., assign to the blank identifier) so
the error is considered handled; apply this to all modified fmt.Fprintln/Fprintf
invocations in that snippet so CI passes.
In `@internal/mcpinit/status.go`:
- Line 88: Update the stale comment numbering for the project-redirects step in
internal/mcpinit/status.go: change the comment text "// 6. Project redirects."
to "// 7. Project redirects." to reflect the inserted autoMemoryEnabled check
that pushed the settings checks to 4-6; locate the comment near the settings
validation block where steps are enumerated and adjust only the numeric label.
---
Nitpick comments:
In `@internal/mcpinit/init_test.go`:
- Around line 160-253: Add a new unit test that covers the dry-run branch when
the current setting is true: create a temp HOME and settings.json containing
{"autoMemoryEnabled":true}, call loadSettings and then ensureAutoMemoryDisabled
with dryRun=true, assert the output contains "would set autoMemoryEnabled:
false", and assert the in-memory setting (sf.getAutoMemoryEnabled()) remains
true (i.e., dry run does not mutate state). Use the existing test patterns in
TestEnsureAutoMemoryDisabled_DryRun to locate usage of loadSettings,
ensureAutoMemoryDisabled, and sf.getAutoMemoryEnabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 16758301-f3f6-42e6-862c-457b8b283ed7
📒 Files selected for processing (6)
README.mdinternal/mcpinit/init.gointernal/mcpinit/init_test.gointernal/mcpinit/settings.gointernal/mcpinit/settings_test.gointernal/mcpinit/status.go
Signed-off-by: wcatz <waynecataldo@gmail.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/mcpinit/init.go (1)
47-75:⚠️ Potential issue | 🔴 CriticalFix CI: unchecked
fmt.Fprintlnerrors are failinggolangci-lint(errcheck).The CI pipeline fails on lines 47, 53, and 68 because these
fmt.Fprintlncalls don't discard their return values, while lines 27/34/40 in this same function use the_, _ =form. The helper at lines 232/234/244/246 has the same inconsistency. Either prefix with_, _ =(matching the pattern already established in this file) or usefmt.Fprintf(w, "...\n", ...)consistently.🛠 Proposed fix to silence errcheck on the new/touched lines
- fmt.Fprintln(w, "\n[4/7] Configuring SessionStart hook...") + _, _ = fmt.Fprintln(w, "\n[4/7] Configuring SessionStart hook...") @@ - // Step 5: Disable Claude Code's built-in file memory. - fmt.Fprintln(w, "\n[5/7] Disabling Claude Code built-in memory...") + // Step 5: Disable Claude Code's built-in file memory. + _, _ = fmt.Fprintln(w, "\n[5/7] Disabling Claude Code built-in memory...") @@ - // Step 6: Import Claude Code memories. - fmt.Fprintln(w, "\n[6/7] Importing Claude Code memories...") + // Step 6: Import Claude Code memories. + _, _ = fmt.Fprintln(w, "\n[6/7] Importing Claude Code memories...") @@ - // Step 7: Project memory redirects. - fmt.Fprintln(w, "\n[7/7] Writing project memory redirects...") + // Step 7: Project memory redirects. + _, _ = fmt.Fprintln(w, "\n[7/7] Writing project memory redirects...")And in
ensureAutoMemoryDisabled:- fmt.Fprintln(w, " ~ would set autoMemoryEnabled: false") + _, _ = fmt.Fprintln(w, " ~ would set autoMemoryEnabled: false") } else { - fmt.Fprintln(w, " ✓ autoMemoryEnabled already false") + _, _ = fmt.Fprintln(w, " ✓ autoMemoryEnabled already false") } @@ - fmt.Fprintln(w, " + set autoMemoryEnabled: false (disables competing file-memory)") + _, _ = fmt.Fprintln(w, " + set autoMemoryEnabled: false (disables competing file-memory)") } else { - fmt.Fprintln(w, " ✓ autoMemoryEnabled already false") + _, _ = fmt.Fprintln(w, " ✓ autoMemoryEnabled already false") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/mcpinit/init.go` around lines 47 - 75, Several fmt.Fprintln calls in init.go are ignoring their returned (int, error) values which fails errcheck; update the calls at the SessionStart/configure, disabling memory, and importing memories sections (the fmt.Fprintln calls around the calls to ensureHook, ensureAutoMemoryDisabled, and before importMemories) to explicitly discard returns using the existing file pattern (e.g., "_, _ = fmt.Fprintln(...)"). Also apply the same change to the helper fmt.Fprintln calls referenced near the helpers (the lines around ensureAutoMemoryDisabled and related helpers) so all fmt.Fprintln usage in this file consistently uses "_, _ =" to silence errcheck.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@internal/mcpinit/init.go`:
- Around line 47-75: Several fmt.Fprintln calls in init.go are ignoring their
returned (int, error) values which fails errcheck; update the calls at the
SessionStart/configure, disabling memory, and importing memories sections (the
fmt.Fprintln calls around the calls to ensureHook, ensureAutoMemoryDisabled, and
before importMemories) to explicitly discard returns using the existing file
pattern (e.g., "_, _ = fmt.Fprintln(...)"). Also apply the same change to the
helper fmt.Fprintln calls referenced near the helpers (the lines around
ensureAutoMemoryDisabled and related helpers) so all fmt.Fprintln usage in this
file consistently uses "_, _ =" to silence errcheck.
Signed-off-by: wcatz <waynecataldo@gmail.com>
Problem
Claude Code maintains its own flat-file memory system under
~/.claude/projects/*/memory/. When Ghost is installed alongside it, both systems compete:autoMemoryEnabled, which defaults totrue)This was discovered in a real session where stale file-memory was overriding Ghost context and causing incorrect behavior.
Fix
ghost mcp initnow includes a new step 5/7 that sets"autoMemoryEnabled": falsein~/.claude/settings.json, stopping Claude Code from writing to its own file-memory during sessions where Ghost is active.What changes
settings.go:setAutoMemoryEnabled(v bool)/getAutoMemoryEnabled()— idempotent read-modify-write onsettingsFile; merges with all existing keys, never clobbersinit.go:ensureAutoMemoryDisabled()added as step 5/7; dry-run aware; rides the same atomic save used by the permissions and hook stepsstatus.go:ghost mcp statusnow surfaces a check forautoMemoryEnabled: falseREADME.md: step table updated (6→7), new step documentedProperties
ghost mcp initwhen the flag is alreadyfalseis a no-opautoMemoryEnabledkey is touched; all other settings are preserved--dry-run— reports what would change without writing anythingTests added
settings_test.gocases: absent key, idempotent (false→false), override (true→false), key-preservation, round-trip,getAutoMemoryEnabledabsentinit_test.gocases: sets flag, idempotent, dry-runOut of scope
Aggressively cleaning up existing
~/.claude/projects/*/memory/directories is intentionally deferred. Ghost's step 7 continues to writeMEMORY.mdredirects into those dirs to guide Claude toward Ghost; removing the dirs would break that mechanism.Test plan
go test ./internal/mcpinit/...— all 38 tests passgo test ./...— full suite greengo vet ./...— cleanghost mcp initshows[5/7] Disabling Claude Code built-in memory... + set autoMemoryEnabled: false✓ autoMemoryEnabled already falseghost mcp statusreports the new checkSummary by CodeRabbit
New Features
Documentation
Bug Fixes / Health Checks