feat(mcp): phase 1 — new tools, resources, enriched hook#136
Merged
Conversation
Add 3 new MCP tools using existing store methods:
- ghost_memory_pin: pin/unpin memories (uses TogglePin)
- ghost_task_update: update task status/priority/desc (uses UpdateTask)
- ghost_decisions_list: list project decisions (uses ListDecisions)
Add category filter to ghost_memory_search (post-filter, no
interface changes).
Add 2 new pinnable MCP resources:
- ghost://project/{project_id}/decisions
- ghost://project/{project_id}/tasks
Enrich SessionStart hook with:
- Open tasks (pending/active/blocked)
- Recent active decisions
- Interaction count (Session #N)
Update ghostPermissions from 13 to 16 entries.
📝 WalkthroughWalkthroughAdds session context (tasks, decisions, interaction count) to the init hook output, extends loadSessionContext DB reads, registers three new MCP tools (memory_pin, task_update, decisions_list), adds two MCP resource templates for project tasks/decisions, and extends memory search with optional category filtering. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCP_Server
participant Store
Note over Client,MCP_Server: Task Update Flow
Client->>MCP_Server: ghost_task_update(task_id,status,priority)
MCP_Server->>MCP_Server: Validate inputs (task_id,status in {pending,active,blocked,done}, clamp priority)
MCP_Server->>Store: UpdateTask(task_id, status, priority, description?)
Store-->>MCP_Server: Success/Failure
MCP_Server-->>Client: Confirmation / Error
sequenceDiagram
participant Client
participant MCP_Server
participant Store
Note over Client,MCP_Server: Decisions List Flow
Client->>MCP_Server: ghost_decisions_list(project_id, limit)
MCP_Server->>MCP_Server: Normalize limit, resolve project_id
MCP_Server->>Store: ListDecisions(project, status='active', limit)
Store-->>MCP_Server: Decisions[]
MCP_Server->>MCP_Server: Format markdown list or empty message
MCP_Server-->>Client: Formatted decisions / "No decisions found"
sequenceDiagram
participant Client
participant MCP_Server
participant Store
Note over Client,MCP_Server: Memory Pin Toggle Flow
Client->>MCP_Server: ghost_memory_pin(memory_id, pinned?)
MCP_Server->>MCP_Server: Validate memory_id
MCP_Server->>Store: TogglePin(memory_id, pinned)
Store-->>MCP_Server: New pin state
MCP_Server-->>Client: Confirmation message
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 1
🤖 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 214-235: The post-filtering logic can return up to searchLimit
results (e.g., args.Limit * 3) and does not enforce the original args.Limit when
args.Category is set; after filtering the slice `memories` (results from
`s.store.SearchHybrid`) you should truncate the filtered slice to at most
`args.Limit` (e.g., by reslicing to `filtered[:min(len(filtered), args.Limit)]`)
so the handler returns no more than the requested number of items when
`args.Category` is provided.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c099ad96-e405-43c2-a0f9-67936aeb82c8
📒 Files selected for processing (4)
internal/mcpinit/hook.gointernal/mcpinit/init_test.gointernal/mcpinit/settings.gointernal/mcpserver/mcpserver.go
CodeRabbit finding: when category filter is set, over-fetched results were not truncated back to args.Limit after filtering.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 1016-1019: The loop calling s.store.ListTasks for statuses
("active","blocked","pending") currently swallows errors (continues on err)
which causes DB/read failures to be treated as "No open tasks"; change the error
handling in the ListTasks calls so that instead of continue you return or
propagate a wrapped error (or at minimum log the error and return a
5xx/appropriate error response) to the caller/HTTP response so real store
failures are not masked; update the code paths around s.store.ListTasks,
projectID and the logic that builds the "No open tasks for this project."
response to propagate the ListTasks error (or surface a clear server error)
rather than ignoring it.
- Around line 787-823: The tool accepts omitted Priority and Description as
zero-values which clobbers existing DB values; change taskUpdateArgs.Priority
and .Description to pointer types (e.g., *int and *string) and update the call
site and store API accordingly so UpdateTask can distinguish omitted vs
intended-zero values; either (A) change TaskStore.UpdateTask signature (and
internal/memory/tasks.go implementation) to accept pointer types and perform
selective column updates only when pointers are non-nil, or (B) add a new method
like UpdateTaskFields(ctx, id string, status string, priority *int, description
*string) used by the ghost_task_update handler and implemented in
internal/memory/tasks.go to read-and-update only provided fields. Ensure input
validation uses nil checks (e.g., only validate/normalize priority when non-nil)
and update the handler to pass pointers from taskUpdateArgs.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: eed2743f-c007-4e1e-a7b1-4a3f161deeb5
📒 Files selected for processing (1)
internal/mcpserver/mcpserver.go
Comment on lines
+787
to
+823
| type taskUpdateArgs struct { | ||
| TaskID string `json:"task_id" jsonschema:"Task ID to update"` | ||
| Status string `json:"status" jsonschema:"New status: pending, active, blocked, done"` | ||
| Priority int `json:"priority,omitempty" jsonschema:"Priority 0-4 (0=critical, 2=normal, 4=low)"` | ||
| Description string `json:"description,omitempty" jsonschema:"Updated description"` | ||
| } | ||
|
|
||
| mcp.AddTool(s.mcp, &mcp.Tool{ | ||
| Name: "ghost_task_update", | ||
| Description: "Update a task's status, priority, or description. Use to change status to active/blocked, reprioritize, or refine the description.", | ||
| Annotations: &mcp.ToolAnnotations{ | ||
| DestructiveHint: boolPtr(false), | ||
| IdempotentHint: true, | ||
| OpenWorldHint: boolPtr(false), | ||
| }, | ||
| }, func(ctx context.Context, req *mcp.CallToolRequest, args taskUpdateArgs) (*mcp.CallToolResult, any, error) { | ||
| if args.TaskID == "" { | ||
| return nil, nil, fmt.Errorf("task_id is required") | ||
| } | ||
| if args.Status == "" { | ||
| return nil, nil, fmt.Errorf("status is required (pending, active, blocked, done)") | ||
| } | ||
| validStatuses := map[string]bool{ | ||
| "pending": true, "active": true, "blocked": true, "done": true, | ||
| } | ||
| if !validStatuses[args.Status] { | ||
| return nil, nil, fmt.Errorf("invalid status %q — must be one of: pending, active, blocked, done", args.Status) | ||
| } | ||
| if args.Priority < 0 || args.Priority > 4 { | ||
| args.Priority = 2 | ||
| } | ||
| if err := s.store.UpdateTask(ctx, args.TaskID, args.Status, args.Priority, args.Description); err != nil { | ||
| return nil, nil, fmt.Errorf("update task: %w", err) | ||
| } | ||
| return &mcp.CallToolResult{ | ||
| Content: []mcp.Content{&mcp.TextContent{Text: "Task updated."}}, | ||
| }, nil, nil |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
sed -n '787,823p' internal/mcpserver/mcpserver.go
fd '^tasks\.go$' internal --exec sed -n '94,110p' {}
rg -nP --type=go '\b(GetTask|FindTask|LookupTask)\s*\(' internalRepository: wcatz/ghost
Length of output: 2193
🏁 Script executed:
# Search more broadly for any Store method that reads a task
rg -n 'func \(s \*Store\).*Task.*\(' internal/memory/tasks.go
# Check Go version requirement
cat go.mod | head -5
# Run go vet
go vet ./...Repository: wcatz/ghost
Length of output: 5506
🏁 Script executed:
# Search for other UpdateTask calls
rg -n 'UpdateTask' --type=go
# Check if this is a CLI application
fd 'main\.go' --type f | head -5
# Check the cmd structure
ls -la cmd/ 2>/dev/null || echo "No cmd directory"Repository: wcatz/ghost
Length of output: 1051
🏁 Script executed:
# Check the actual test to see if it tests partial updates
sed -n '1180,1200p' internal/memory/store_test.go
# Quick check - does UpdateTask work as described (partial updates)?
rg -B5 -A5 'UpdateTask.*active' internalRepository: wcatz/ghost
Length of output: 1280
Omitted fields in task update will clobber existing values
The Priority and Description fields in taskUpdateArgs are non-pointer types, so omitted values arrive as 0 and "". Line 818 passes these directly to UpdateTask, which unconditionally updates all columns in the database (lines 95–105 in internal/memory/tasks.go). A status-only update will reset priority to 0 and clear the description.
The tool description claims it can update "status, priority, or description" in any combination, but the implementation doesn't support partial updates. Additionally, no GetTask method exists to enable the read-modify-write pattern needed to preserve omitted fields.
Either redesign this as a full-replace API (all three fields required), make Priority and Description pointers (with corresponding changes to the TaskStore interface), or add a task read method and implement selective updates.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/mcpserver/mcpserver.go` around lines 787 - 823, The tool accepts
omitted Priority and Description as zero-values which clobbers existing DB
values; change taskUpdateArgs.Priority and .Description to pointer types (e.g.,
*int and *string) and update the call site and store API accordingly so
UpdateTask can distinguish omitted vs intended-zero values; either (A) change
TaskStore.UpdateTask signature (and internal/memory/tasks.go implementation) to
accept pointer types and perform selective column updates only when pointers are
non-nil, or (B) add a new method like UpdateTaskFields(ctx, id string, status
string, priority *int, description *string) used by the ghost_task_update
handler and implemented in internal/memory/tasks.go to read-and-update only
provided fields. Ensure input validation uses nil checks (e.g., only
validate/normalize priority when non-nil) and update the handler to pass
pointers from taskUpdateArgs.
Comment on lines
+1016
to
+1019
| for _, status := range []string{"active", "blocked", "pending"} { | ||
| tasks, err := s.store.ListTasks(ctx, projectID, status, 15) | ||
| if err != nil { | ||
| continue |
There was a problem hiding this comment.
Don't turn store failures into “No open tasks.”
Line 1018 and Line 1019 swallow ListTasks errors, so a DB/read failure becomes a partial list or the misleading "No open tasks for this project." response built on Line 1031. Since this resource is meant to be pinned into session context, hiding the failure drops active work on the floor.
🔧 Minimal fix
for _, status := range []string{"active", "blocked", "pending"} {
tasks, err := s.store.ListTasks(ctx, projectID, status, 15)
if err != nil {
- continue
+ return nil, fmt.Errorf("list %s tasks for %q: %w", status, projectID, err)
}
for _, t := range tasks {Also applies to: 1030-1035
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/mcpserver/mcpserver.go` around lines 1016 - 1019, The loop calling
s.store.ListTasks for statuses ("active","blocked","pending") currently swallows
errors (continues on err) which causes DB/read failures to be treated as "No
open tasks"; change the error handling in the ListTasks calls so that instead of
continue you return or propagate a wrapped error (or at minimum log the error
and return a 5xx/appropriate error response) to the caller/HTTP response so real
store failures are not masked; update the code paths around s.store.ListTasks,
projectID and the logic that builds the "No open tasks for this project."
response to propagate the ListTasks error (or surface a clear server error)
rather than ignoring it.
This was referenced Mar 31, 2026
5 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
Exposes existing store capabilities via MCP without schema changes.
3 new tools (16 total):
ghost_memory_pin— pin/unpin memoriesghost_task_update— update task status/priority/descriptionghost_decisions_list— list recorded decisions with rationaleCategory filter on
ghost_memory_search— optionalcategoryparam, post-filter approach (no interface changes)2 new pinnable resources (4 total):
ghost://project/{project_id}/decisions— active decisionsghost://project/{project_id}/tasks— open tasksEnriched SessionStart hook — now injects:
Test plan
go vet ./...cleango test ./...all passecho '{"cwd":"..."}' | ghost hook session-startshows session countSummary by CodeRabbit
New Features
Tests
Chores