Skip to content

fix(mcp): trust process project override#378

Merged
Alan-TheGentleman merged 2 commits into
mainfrom
fix/mcp-project-override
May 14, 2026
Merged

fix(mcp): trust process project override#378
Alan-TheGentleman merged 2 commits into
mainfrom
fix/mcp-project-override

Conversation

@Alan-TheGentleman
Copy link
Copy Markdown
Collaborator

Summary

  • wire engram mcp --project and ENGRAM_PROJECT into MCP process config
  • use the trusted process project before cwd detection for current project, read resolution, and write resolution
  • keep explicit per-call project validation strict, including explicit empty project errors

Fixes #312
Fixes #248
Fixes #347

Non-goals

Tests

  • go test ./internal/mcp ./cmd/engram
  • go test ./...

Review

  • Fresh reviewer found one strict-validation blocker; fixed and re-reviewed approved.

Copilot AI review requested due to automatic review settings May 14, 2026 11:33
@Alan-TheGentleman Alan-TheGentleman added the type:bug Bug fix label May 14, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR restores a process-level project override for the MCP server so long-lived MCP hosts can reliably scope reads/writes to a configured project (via engram mcp --project or ENGRAM_PROJECT) instead of being forced to rely on cwd-based detection.

Changes:

  • Add MCPConfig.DefaultProject and thread it through MCP tool handlers to take precedence over cwd detection for current-project, read resolution, and write resolution.
  • Wire --project and ENGRAM_PROJECT into cmdMCP and pass them into the MCP server config.
  • Update/extend tests to cover the new process-override precedence and to keep per-call project validation strict.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
internal/mcp/mcp.go Introduces DefaultProject process override, adds helpers to apply it before cwd detection across read/write resolution and selected tools.
internal/mcp/mcp_test.go Updates handler call signatures and adds tests asserting override precedence and validation invariants.
cmd/engram/main.go Parses --project and ENGRAM_PROJECT for engram mcp and passes the override into mcp.MCPConfig.
cmd/engram/main_test.go Updates tests to assert the new DefaultProject wiring from flag/env.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/mcp/mcp.go
Comment on lines +2036 to +2047
func processProjectResult(project string) (projectpkg.DetectionResult, bool) {
project = strings.TrimSpace(project)
if project == "" {
return projectpkg.DetectionResult{}, false
}
normalized, warning := store.NormalizeProject(project)
return projectpkg.DetectionResult{
Project: normalized,
Source: sourceProcessOverride,
Path: "",
Warning: warning,
}, true
Comment thread internal/mcp/mcp.go
Comment on lines 872 to +874
res := projectpkg.DetectProjectFull(cwd)
if processRes, ok := processProjectResult(cfg.DefaultProject); ok {
res = processRes
Comment thread cmd/engram/main.go
Comment on lines 832 to 855
func cmdMCP(cfg store.Config) {
// Parse --tools flag. Project is always auto-detected from cwd at call time (JR2-4).
toolsFilter := ""
projectOverride := strings.TrimSpace(os.Getenv("ENGRAM_PROJECT"))
for i := 2; i < len(os.Args); i++ {
if strings.HasPrefix(os.Args[i], "--tools=") {
toolsFilter = strings.TrimPrefix(os.Args[i], "--tools=")
} else if os.Args[i] == "--tools" && i+1 < len(os.Args) {
toolsFilter = os.Args[i+1]
i++
} else if strings.HasPrefix(os.Args[i], "--project=") {
projectOverride = strings.TrimSpace(strings.TrimPrefix(os.Args[i], "--project="))
if projectOverride == "" {
fatal(fmt.Errorf("--project requires a value"))
}
} else if os.Args[i] == "--project" {
if i+1 >= len(os.Args) {
fatal(fmt.Errorf("--project requires a value"))
}
projectOverride = strings.TrimSpace(os.Args[i+1])
if projectOverride == "" {
fatal(fmt.Errorf("--project requires a value"))
}
i++
}
@Alan-TheGentleman Alan-TheGentleman merged commit b094052 into main May 14, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment