fix(mcp): audit fixes — stale instructions, validation, safety gaps#129
Merged
fix(mcp): audit fixes — stale instructions, validation, safety gaps#129
Conversation
…y gaps - Update MCP instructions: stop telling Claude to call tools at session start (hook injects context automatically) - Add global memories to buildProjectContext - Add category validation to ghost_save_global - Cap Limit parameters at 100 - Pass version to MCP server (was hardcoded 0.1.0) - Fix hook fallback, add symlink resolution, UTF-8 safe truncation - Fix stale test assertions
📝 WalkthroughWalkthroughThe PR threads a Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/mcpinit/hook.go (1)
34-43:⚠️ Potential issue | 🟠 MajorDon't canonicalize only on the read path.
Line 39 resolves
cwd, butinternal/memory/store.go, Lines 154-171, still persistprojects.pathexactly as provided. A project first registered through a symlinked workspace now drops to the weakername = cwdBaselookup here, which can miss or inject the wrong project's memories when leaf names collide.Possible stopgap in this handler
cwd := input.CWD if cwd == "" { cwd, _ = os.Getwd() } + rawCWD := cwd // Resolve symlinks so cwd matches the canonical path stored in the DB. if resolved, err := filepath.EvalSymlinks(cwd); err == nil { cwd = resolved } project, memories, learned := loadSessionContext(cwd) + if project == "" && rawCWD != cwd { + project, memories, learned = loadSessionContext(rawCWD) + }Longer-term,
EnsureProjectshould normalize paths before persisting them.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/mcpinit/hook.go` around lines 34 - 43, The handler canonicalizes cwd only on read (filepath.EvalSymlinks in loadSessionContext), causing mismatches when projects were persisted with symlinked paths; update this handler to canonicalize input.CWD before any lookup and ensure project creation/update uses the canonical path: call filepath.EvalSymlinks (and fallback to os.Getwd when empty) before invoking loadSessionContext/EnsureProject so the same normalized path is used for both lookup and persistence; also update EnsureProject in internal/memory/store.go to normalize incoming project paths before saving to projects.path so persisted paths are always canonicalized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/mcpserver/mcpserver.go`:
- Around line 301-304: The ghost_project_context tool handler currently builds
its response from project-only memories; update the handler(s) that assemble the
tool response (the ghost_project_context tool handler around the
buildProjectContext call and the duplicate assembly at the later occurrence) to
include the `_global` memories that buildProjectContext now returns—i.e., call
buildProjectContext (or use its returned struct) and merge both project memories
and the returned global memories into the tool response so globals aren't
dropped during mid-session refreshes and project switches.
- Around line 67-75: The startup instructions in the "Workflow" block are not
client-agnostic and claim behavior specific to Claude Code; edit the text (the
"Workflow" block and sentences mentioning auto-import) to make it
conditional/neutral: reference the SessionStart hook generically and remove the
absolute claim that "Line 68 is only true for Claude Code"; instead say "If a
client implements the SessionStart hook (e.g., Claude Code), the project context
may be injected — otherwise clients should call ghost_project_context as
needed." Also change the "Ghost auto-imports Claude Code memory files..."
sentence to a conditional note like "Some clients may auto-import memory files
(for example Claude Code when the SessionStart hook is present)." Ensure
references to ghost_project_context, SessionStart hook, and
ghost_memory_save/search remain unchanged so behavior guidance is preserved.
---
Outside diff comments:
In `@internal/mcpinit/hook.go`:
- Around line 34-43: The handler canonicalizes cwd only on read
(filepath.EvalSymlinks in loadSessionContext), causing mismatches when projects
were persisted with symlinked paths; update this handler to canonicalize
input.CWD before any lookup and ensure project creation/update uses the
canonical path: call filepath.EvalSymlinks (and fallback to os.Getwd when empty)
before invoking loadSessionContext/EnsureProject so the same normalized path is
used for both lookup and persistence; also update EnsureProject in
internal/memory/store.go to normalize incoming project paths before saving to
projects.path so persisted paths are always canonicalized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65b52cde-0551-4a17-8534-3480c7ea5aea
📒 Files selected for processing (6)
README.mdcmd/ghost/main.gointernal/mcpinit/hook.gointernal/mcpinit/init_test.gointernal/mcpserver/mcpserver.gointernal/mcpserver/mcpserver_test.go
Comment on lines
67
to
+75
| ## Workflow | ||
| 1. At session start, call ghost_list_projects to discover known projects. | ||
| 2. Call ghost_project_context with the project name to load accumulated knowledge. | ||
| 3. During work, save important discoveries with ghost_memory_save. Do NOT wait until the end of the session. | ||
| 4. Use ghost_memory_search to recall specific facts. | ||
| 5. No special action needed at session end — Ghost persists automatically. | ||
| 1. At session start, Ghost's SessionStart hook has ALREADY injected your project context (memories + global preferences) into this conversation. READ IT — do not call ghost_project_context redundantly. | ||
| 2. During work, save important discoveries with ghost_memory_save. Do NOT wait until the end of the session. | ||
| 3. Use ghost_memory_search to recall specific facts not in the injected context. | ||
| 4. No special action needed at session end — Ghost persists automatically. | ||
|
|
||
| Ghost auto-imports Claude Code memory files (~/.claude/projects/*/memory/*.md) on first project contact. No manual migration is needed. Ghost has built-in upsert/merge deduplication — it is always safe to save, even if similar knowledge already exists. | ||
|
|
||
| IMPORTANT: Global memories (preferences, conventions) are included in the SessionStart hook output under "Global (applies to all projects)". These are non-negotiable rules from the user. Follow them. |
There was a problem hiding this comment.
Keep the startup instructions client-agnostic.
Line 68 is only true for Claude Code with the session-start hook installed. These instructions are emitted by the MCP server itself, so any non-Claude client that honors them will skip its initial ghost_project_context call and start without project context.
Suggested wording
-1. At session start, Ghost's SessionStart hook has ALREADY injected your project context (memories + global preferences) into this conversation. READ IT — do not call ghost_project_context redundantly.
-2. During work, save important discoveries with ghost_memory_save. Do NOT wait until the end of the session.
-3. Use ghost_memory_search to recall specific facts not in the injected context.
-4. No special action needed at session end — Ghost persists automatically.
+1. If the client already injected Ghost SessionStart context, read it and do not call ghost_project_context redundantly.
+2. Otherwise, call ghost_project_context once at session start to load project context.
+3. During work, save important discoveries with ghost_memory_save. Do NOT wait until the end of the session.
+4. Use ghost_memory_search to recall specific facts not already in context.
+5. No special action needed at session end — Ghost persists automatically.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/mcpserver/mcpserver.go` around lines 67 - 75, The startup
instructions in the "Workflow" block are not client-agnostic and claim behavior
specific to Claude Code; edit the text (the "Workflow" block and sentences
mentioning auto-import) to make it conditional/neutral: reference the
SessionStart hook generically and remove the absolute claim that "Line 68 is
only true for Claude Code"; instead say "If a client implements the SessionStart
hook (e.g., Claude Code), the project context may be injected — otherwise
clients should call ghost_project_context as needed." Also change the "Ghost
auto-imports Claude Code memory files..." sentence to a conditional note like
"Some clients may auto-import memory files (for example Claude Code when the
SessionStart hook is present)." Ensure references to ghost_project_context,
SessionStart hook, and ghost_memory_save/search remain unchanged so behavior
guidance is preserved.
Comment on lines
301
to
304
| mcp.AddTool(s.mcp, &mcp.Tool{ | ||
| Name: "ghost_project_context", | ||
| Description: "Get Ghost's accumulated knowledge about a project: top memories ranked by importance and recency, plus any learned context summaries. Use this at the start of a session to recall what Ghost knows.", | ||
| Description: "Get Ghost's accumulated knowledge about a project: top memories ranked by importance and recency, plus global memories and learned context. Usually NOT needed at session start — the SessionStart hook already injects this. Use when switching projects mid-session or refreshing after saves.", | ||
| Annotations: &mcp.ToolAnnotations{ |
There was a problem hiding this comment.
Return globals from ghost_project_context too.
These changes add _global memories to buildProjectContext, but the ghost_project_context tool handler still assembles its own response from project memories only at Lines 335-351. Mid-session refreshes and project switches will therefore drop the same global preferences that SessionStart and resource reads now include.
Also applies to: 833-840
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/mcpserver/mcpserver.go` around lines 301 - 304, The
ghost_project_context tool handler currently builds its response from
project-only memories; update the handler(s) that assemble the tool response
(the ghost_project_context tool handler around the buildProjectContext call and
the duplicate assembly at the later occurrence) to include the `_global`
memories that buildProjectContext now returns—i.e., call buildProjectContext (or
use its returned struct) and merge both project memories and the returned global
memories into the tool response so globals aren't dropped during mid-session
refreshes and project switches.
This was referenced Mar 27, 2026
3 tasks
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
1 participant
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
ghost_project_contextandghost_list_projectsdescriptions updatedbuildProjectContextnow includes_globalmemoriesghost_save_globalgets category validation (was missing)Test plan
go test ./...passesgo vet ./...cleanSummary by CodeRabbit
New Features
Bug Fixes
Documentation