Skip to content

Commit 21f530e

Browse files
fix(mcp): recover ambiguous project writes
1 parent 9f697c8 commit 21f530e

4 files changed

Lines changed: 257 additions & 11 deletions

File tree

docs/AGENT-SETUP.md

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ Engram works with **any MCP-compatible agent**. Pick your agent below.
2727

2828
### Project auto-detection (important)
2929

30-
**Do not pass `project` to write tools.** Engram auto-detects the project from the server's working directory (cwd) using `.engram/config.json`, git remote URL, repo root name, or directory basename. Agents that include `project` in `mem_save` or similar calls will have that argument silently discarded.
30+
**Do not pass `project` to write tools during normal operation.** Engram auto-detects the project from the server's working directory (cwd) using `.engram/config.json`, git remote URL, repo root name, or directory basename. Agents that include `project` in `mem_save` or similar calls will have that argument ignored unless they are using the explicit ambiguous-project recovery flow below.
3131

3232
To lock write tools to the canonical project for a repo, add `.engram/config.json` at the repo root:
3333

@@ -41,6 +41,17 @@ When present, `project_name` is used for writes from the repo and its subdirecto
4141

4242
**Recommended first call:** `mem_current_project` — confirms which project Engram detected before you start writing. Returns `project_source` (how it was detected) and `available_projects` (if cwd is ambiguous).
4343

44+
If a write tool returns `ambiguous_project`, the agent must not guess. Ask the user to choose one of `available_projects`, then retry only `mem_save` or `mem_save_prompt` with both fields:
45+
46+
```json
47+
{
48+
"project": "chosen-project-from-available-projects",
49+
"project_choice_reason": "user_selected_after_ambiguous_project"
50+
}
51+
```
52+
53+
This recovery path is accepted only after cwd detection is ambiguous and only when `project` exactly matches one of the reported `available_projects`. In all non-ambiguous cases, `.engram/config.json`/git/cwd detection remains authoritative and the explicit `project` is ignored. Alternatives: `cd` into the target repo before starting the MCP server, or add repo `.engram/config.json`.
54+
4455
**Read tools** (`mem_search`, `mem_context`, `mem_get_observation`, `mem_stats`, `mem_timeline`) accept an optional `project` override validated against the store. Omit it to auto-detect.
4556

4657
---

internal/mcp/mcp.go

Lines changed: 80 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,12 @@ Examples:
328328
mcp.WithString("topic_key",
329329
mcp.Description("Optional topic identifier for upserts (e.g. architecture/auth-model). Reuses and updates the latest observation in same project+scope."),
330330
),
331+
mcp.WithString("project",
332+
mcp.Description("Optional recovery target only after ambiguous_project. Ignored unless project_choice_reason is user_selected_after_ambiguous_project."),
333+
),
334+
mcp.WithString("project_choice_reason",
335+
mcp.Description("Must be user_selected_after_ambiguous_project, and only after the user explicitly chose one of available_projects from an ambiguous_project error."),
336+
),
331337
),
332338
queuedWriteHandler(writeQueue, handleSave(s, cfg, activity)),
333339
)
@@ -433,6 +439,12 @@ Examples:
433439
mcp.WithString("session_id",
434440
mcp.Description("Session ID to associate with (default: manual-save-{project})"),
435441
),
442+
mcp.WithString("project",
443+
mcp.Description("Optional recovery target only after ambiguous_project. Ignored unless project_choice_reason is user_selected_after_ambiguous_project."),
444+
),
445+
mcp.WithString("project_choice_reason",
446+
mcp.Description("Must be user_selected_after_ambiguous_project, and only after the user explicitly chose one of available_projects from an ambiguous_project error."),
447+
),
436448
),
437449
queuedWriteHandler(writeQueue, handleSavePrompt(s, cfg)),
438450
)
@@ -996,10 +1008,12 @@ func handleSave(s *store.Store, cfg MCPConfig, activity *SessionActivity) server
9961008
sessionID, _ := req.GetArguments()["session_id"].(string)
9971009
scope, _ := req.GetArguments()["scope"].(string)
9981010
topicKey, _ := req.GetArguments()["topic_key"].(string)
999-
// project field intentionally not read — auto-detect only (REQ-308)
1011+
projectChoice, _ := req.GetArguments()["project"].(string)
1012+
projectChoiceReason, _ := req.GetArguments()["project_choice_reason"].(string)
10001013

1001-
// Auto-detect project from cwd; fail fast on ambiguous (REQ-308, REQ-309)
1002-
detRes, err := resolveWriteProject()
1014+
// Auto-detect project from cwd; only allow explicit user-selected recovery
1015+
// after ErrAmbiguousProject (issue #306).
1016+
detRes, err := resolveWriteProjectWithChoice(projectChoice, projectChoiceReason)
10031017
if err != nil {
10041018
return writeProjectErrorResult(detRes, err), nil
10051019
}
@@ -1232,9 +1246,10 @@ func handleSavePrompt(s *store.Store, cfg MCPConfig) server.ToolHandlerFunc {
12321246
return func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) {
12331247
content, _ := req.GetArguments()["content"].(string)
12341248
sessionID, _ := req.GetArguments()["session_id"].(string)
1235-
// project field intentionally not read — auto-detect only (REQ-308)
1249+
projectChoice, _ := req.GetArguments()["project"].(string)
1250+
projectChoiceReason, _ := req.GetArguments()["project_choice_reason"].(string)
12361251

1237-
detRes, err := resolveWriteProject()
1252+
detRes, err := resolveWriteProjectWithChoice(projectChoice, projectChoiceReason)
12381253
if err != nil {
12391254
return writeProjectErrorResult(detRes, err), nil
12401255
}
@@ -1866,6 +1881,15 @@ func (e *unknownProjectError) Error() string {
18661881
return "unknown project: " + e.Name
18671882
}
18681883

1884+
type invalidProjectChoiceError struct {
1885+
Name string
1886+
AvailableProjects []string
1887+
}
1888+
1889+
func (e *invalidProjectChoiceError) Error() string {
1890+
return "invalid project choice: " + e.Name
1891+
}
1892+
18691893
// resolveWriteProject detects the current project from the process working
18701894
// directory. Returns ErrAmbiguousProject if cwd is a parent of multiple repos.
18711895
func resolveWriteProject() (projectpkg.DetectionResult, error) {
@@ -1880,6 +1904,47 @@ func resolveWriteProject() (projectpkg.DetectionResult, error) {
18801904
return res, nil
18811905
}
18821906

1907+
// resolveWriteProjectWithChoice preserves normal write resolution authority and
1908+
// only uses an explicit project choice as a recovery path from ErrAmbiguousProject.
1909+
func resolveWriteProjectWithChoice(projectChoice, reason string) (projectpkg.DetectionResult, error) {
1910+
res, err := resolveWriteProject()
1911+
if err == nil {
1912+
// Non-ambiguous config/git/autodetect remains authoritative. Ignore any
1913+
// supplied project choice so agents cannot drift writes to arbitrary buckets.
1914+
return res, nil
1915+
}
1916+
if !errors.Is(err, projectpkg.ErrAmbiguousProject) {
1917+
return res, err
1918+
}
1919+
1920+
if strings.TrimSpace(reason) != projectpkg.SourceUserSelectedAfterAmbiguousProject {
1921+
return res, err
1922+
}
1923+
1924+
choice, _ := store.NormalizeProject(projectChoice)
1925+
if choice == "" || !containsProjectChoice(res.AvailableProjects, choice) {
1926+
return res, &invalidProjectChoiceError{
1927+
Name: choice,
1928+
AvailableProjects: res.AvailableProjects,
1929+
}
1930+
}
1931+
1932+
res.Project = choice
1933+
res.Source = projectpkg.SourceUserSelectedAfterAmbiguousProject
1934+
res.Warning = "project selected by user after ambiguous_project recovery"
1935+
return res, nil
1936+
}
1937+
1938+
func containsProjectChoice(available []string, choice string) bool {
1939+
for _, candidate := range available {
1940+
normalized, _ := store.NormalizeProject(candidate)
1941+
if normalized == choice {
1942+
return true
1943+
}
1944+
}
1945+
return false
1946+
}
1947+
18831948
// resolveReadProject validates an optional project override against the store.
18841949
// If override is empty, falls back to auto-detection from cwd.
18851950
// JW2: normalizes the override (lowercase+trim) before ProjectExists lookup so
@@ -1934,6 +1999,13 @@ func writeProjectErrorResult(res projectpkg.DetectionResult, err error) *mcp.Cal
19341999
if errors.Is(err, projectpkg.ErrInvalidConfig) {
19352000
code = "invalid_project_config"
19362001
}
2002+
var choiceErr *invalidProjectChoiceError
2003+
if errors.As(err, &choiceErr) {
2004+
return errorWithMeta("invalid_project_choice",
2005+
fmt.Sprintf("Project choice %q is not one of available_projects", choiceErr.Name),
2006+
choiceErr.AvailableProjects,
2007+
)
2008+
}
19372009
return errorWithMeta(code, fmt.Sprintf("Cannot determine project: %s", err), res.AvailableProjects)
19382010
}
19392011

@@ -1947,7 +2019,9 @@ func errorWithMeta(code, msg string, availableProjects []string) *mcp.CallToolRe
19472019
}
19482020
switch code {
19492021
case "ambiguous_project":
1950-
envelope["hint"] = "Use mem_current_project to inspect detection results, or cd into one of the listed repositories."
2022+
envelope["hint"] = "Ask the user to choose one of available_projects, then retry mem_save or mem_save_prompt with project and project_choice_reason=user_selected_after_ambiguous_project; alternatively cd into the target repo or add repo .engram/config.json."
2023+
case "invalid_project_choice":
2024+
envelope["hint"] = "Use exactly one of available_projects after asking the user, or cd into the target repo, or add repo .engram/config.json."
19512025
case "unknown_project":
19522026
envelope["hint"] = "Use one of the available_projects values, or omit project to auto-detect."
19532027
case "invalid_project_config":

internal/mcp/mcp_test.go

Lines changed: 159 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2789,12 +2789,16 @@ func TestSessionStartWithExplicitDirectoryResolvesProjectFromDirectory(t *testin
27892789

27902790
// ─── Batch 4: Write handler schema + auto-detect ─────────────────────────────
27912791

2792-
// TestWriteSchema_NoProjectField asserts that the 6 write tools do NOT include
2793-
// a "project" property in their input schema (REQ-308).
2792+
// TestWriteSchema_ProjectFieldOnlyForAmbiguousRecovery asserts that only the
2793+
// write tools with explicit ambiguous-project recovery expose project fields.
27942794
func TestWriteSchema_NoProjectField(t *testing.T) {
27952795
s := newMCPTestStore(t)
27962796
srv := NewServer(s)
27972797

2798+
recoveryTools := map[string]bool{
2799+
"mem_save": true,
2800+
"mem_save_prompt": true,
2801+
}
27982802
writeTools := []string{
27992803
"mem_save",
28002804
"mem_save_prompt",
@@ -2811,6 +2815,15 @@ func TestWriteSchema_NoProjectField(t *testing.T) {
28112815
t.Fatalf("tool %q not registered", toolName)
28122816
}
28132817
props := st.Tool.InputSchema.Properties
2818+
if recoveryTools[toolName] {
2819+
if _, hasProject := props["project"]; !hasProject {
2820+
t.Errorf("tool %q must expose 'project' for ambiguous-project recovery", toolName)
2821+
}
2822+
if _, hasReason := props["project_choice_reason"]; !hasReason {
2823+
t.Errorf("tool %q must expose 'project_choice_reason' for ambiguous-project recovery", toolName)
2824+
}
2825+
return
2826+
}
28142827
if _, hasProject := props["project"]; hasProject {
28152828
t.Errorf("tool %q must not have 'project' in schema", toolName)
28162829
}
@@ -2931,6 +2944,145 @@ func TestMemSave_AmbiguousEnvelope(t *testing.T) {
29312944
if !strings.Contains(text, "available_projects") {
29322945
t.Errorf("expected available_projects in error, got: %q", text)
29332946
}
2947+
if !strings.Contains(text, "project_choice_reason=user_selected_after_ambiguous_project") {
2948+
t.Errorf("expected explicit recovery hint, got: %q", text)
2949+
}
2950+
}
2951+
2952+
func TestMemSave_AmbiguousWithValidUserChoiceSucceeds(t *testing.T) {
2953+
parent := t.TempDir()
2954+
for _, name := range []string{"repo-choice-a", "repo-choice-b"} {
2955+
child := filepath.Join(parent, name)
2956+
if err := os.MkdirAll(child, 0o755); err != nil {
2957+
t.Fatal(err)
2958+
}
2959+
initTestGitRepo(t, child)
2960+
}
2961+
t.Chdir(parent)
2962+
2963+
s := newMCPTestStore(t)
2964+
h := handleSave(s, MCPConfig{}, NewSessionActivity(10*time.Minute))
2965+
res, err := h(context.Background(), mcppkg.CallToolRequest{Params: mcppkg.CallToolParams{Arguments: map[string]any{
2966+
"title": "chosen project memory",
2967+
"content": "saved after explicit user choice",
2968+
"type": "manual",
2969+
"project": "repo-choice-b",
2970+
"project_choice_reason": project.SourceUserSelectedAfterAmbiguousProject,
2971+
}}})
2972+
if err != nil || res.IsError {
2973+
t.Fatalf("mem_save with choice failed: err=%v isError=%v text=%q", err, res.IsError, callResultText(t, res))
2974+
}
2975+
body := callResultJSON(t, res)
2976+
if body["project"] != "repo-choice-b" || body["project_source"] != project.SourceUserSelectedAfterAmbiguousProject {
2977+
t.Fatalf("expected explicit user choice envelope, got %v", body)
2978+
}
2979+
obs, err := s.Search("chosen project memory", store.SearchOptions{Project: "repo-choice-b", Limit: 5})
2980+
if err != nil || len(obs) != 1 {
2981+
t.Fatalf("expected observation in selected project, obs=%d err=%v", len(obs), err)
2982+
}
2983+
}
2984+
2985+
func TestMemSave_AmbiguousWithInventedProjectRejected(t *testing.T) {
2986+
parent := t.TempDir()
2987+
for _, name := range []string{"repo-valid-a", "repo-valid-b"} {
2988+
child := filepath.Join(parent, name)
2989+
if err := os.MkdirAll(child, 0o755); err != nil {
2990+
t.Fatal(err)
2991+
}
2992+
initTestGitRepo(t, child)
2993+
}
2994+
t.Chdir(parent)
2995+
2996+
s := newMCPTestStore(t)
2997+
h := handleSave(s, MCPConfig{}, NewSessionActivity(10*time.Minute))
2998+
res, err := h(context.Background(), mcppkg.CallToolRequest{Params: mcppkg.CallToolParams{Arguments: map[string]any{
2999+
"title": "invented project memory",
3000+
"content": "must not save",
3001+
"project": "invented-project",
3002+
"project_choice_reason": project.SourceUserSelectedAfterAmbiguousProject,
3003+
}}})
3004+
if err != nil {
3005+
t.Fatalf("handler error: %v", err)
3006+
}
3007+
if !res.IsError {
3008+
t.Fatal("expected invalid project choice error")
3009+
}
3010+
body := callResultJSON(t, res)
3011+
if body["error_code"] != "invalid_project_choice" {
3012+
t.Fatalf("expected invalid_project_choice, got %v", body)
3013+
}
3014+
if strings.Contains(callResultText(t, res), "invented-project\",\"available_projects") {
3015+
t.Fatalf("invented project must not be treated as available: %q", callResultText(t, res))
3016+
}
3017+
obs, err := s.Search("invented project memory", store.SearchOptions{Project: "invented-project", Limit: 5})
3018+
if err != nil || len(obs) != 0 {
3019+
t.Fatalf("invented project must not receive writes, obs=%d err=%v", len(obs), err)
3020+
}
3021+
}
3022+
3023+
func TestMemSavePrompt_AmbiguousWithValidUserChoiceSucceeds(t *testing.T) {
3024+
parent := t.TempDir()
3025+
for _, name := range []string{"repo-prompt-a", "repo-prompt-b"} {
3026+
child := filepath.Join(parent, name)
3027+
if err := os.MkdirAll(child, 0o755); err != nil {
3028+
t.Fatal(err)
3029+
}
3030+
initTestGitRepo(t, child)
3031+
}
3032+
t.Chdir(parent)
3033+
3034+
s := newMCPTestStore(t)
3035+
h := handleSavePrompt(s, MCPConfig{})
3036+
res, err := h(context.Background(), mcppkg.CallToolRequest{Params: mcppkg.CallToolParams{Arguments: map[string]any{
3037+
"content": "prompt after user chose repo-prompt-a",
3038+
"project": "repo-prompt-a",
3039+
"project_choice_reason": project.SourceUserSelectedAfterAmbiguousProject,
3040+
}}})
3041+
if err != nil || res.IsError {
3042+
t.Fatalf("mem_save_prompt with choice failed: err=%v isError=%v text=%q", err, res.IsError, callResultText(t, res))
3043+
}
3044+
body := callResultJSON(t, res)
3045+
if body["project"] != "repo-prompt-a" || body["project_source"] != project.SourceUserSelectedAfterAmbiguousProject {
3046+
t.Fatalf("expected explicit user choice envelope, got %v", body)
3047+
}
3048+
prompts, err := s.RecentPrompts("repo-prompt-a", 5)
3049+
if err != nil || len(prompts) != 1 {
3050+
t.Fatalf("expected prompt in selected project, prompts=%d err=%v", len(prompts), err)
3051+
}
3052+
}
3053+
3054+
func TestMemSavePrompt_AmbiguousWithInventedProjectRejected(t *testing.T) {
3055+
parent := t.TempDir()
3056+
for _, name := range []string{"repo-prompt-valid-a", "repo-prompt-valid-b"} {
3057+
child := filepath.Join(parent, name)
3058+
if err := os.MkdirAll(child, 0o755); err != nil {
3059+
t.Fatal(err)
3060+
}
3061+
initTestGitRepo(t, child)
3062+
}
3063+
t.Chdir(parent)
3064+
3065+
s := newMCPTestStore(t)
3066+
h := handleSavePrompt(s, MCPConfig{})
3067+
res, err := h(context.Background(), mcppkg.CallToolRequest{Params: mcppkg.CallToolParams{Arguments: map[string]any{
3068+
"content": "prompt must not save",
3069+
"project": "invented-prompt-project",
3070+
"project_choice_reason": project.SourceUserSelectedAfterAmbiguousProject,
3071+
}}})
3072+
if err != nil {
3073+
t.Fatalf("handler error: %v", err)
3074+
}
3075+
if !res.IsError {
3076+
t.Fatal("expected invalid project choice error")
3077+
}
3078+
body := callResultJSON(t, res)
3079+
if body["error_code"] != "invalid_project_choice" {
3080+
t.Fatalf("expected invalid_project_choice, got %v", body)
3081+
}
3082+
prompts, err := s.RecentPrompts("invented-prompt-project", 5)
3083+
if err != nil || len(prompts) != 0 {
3084+
t.Fatalf("invented project must not receive prompt, prompts=%d err=%v", len(prompts), err)
3085+
}
29343086
}
29353087

29363088
// TestMemSave_SuccessEnvelope asserts project, project_source, project_path in response (REQ-309).
@@ -3348,6 +3500,7 @@ func TestHandleSaveAndPromptUseConfigProjectForWrites(t *testing.T) {
33483500
save := handleSave(s, MCPConfig{}, NewSessionActivity(10*time.Minute))
33493501
res, err := save(context.Background(), mcppkg.CallToolRequest{Params: mcppkg.CallToolParams{Arguments: map[string]any{
33503502
"title": "config write", "content": "memory saved under config project", "type": "decision",
3503+
"project": "attempted-override", "project_choice_reason": project.SourceUserSelectedAfterAmbiguousProject,
33513504
}}})
33523505
if err != nil || res.IsError {
33533506
t.Fatalf("mem_save failed: err=%v isError=%v text=%q", err, res.IsError, callResultText(t, res))
@@ -3360,6 +3513,7 @@ func TestHandleSaveAndPromptUseConfigProjectForWrites(t *testing.T) {
33603513
prompt := handleSavePrompt(s, MCPConfig{})
33613514
res, err = prompt(context.Background(), mcppkg.CallToolRequest{Params: mcppkg.CallToolParams{Arguments: map[string]any{
33623515
"content": "prompt saved under config project",
3516+
"project": "attempted-override", "project_choice_reason": project.SourceUserSelectedAfterAmbiguousProject,
33633517
}}})
33643518
if err != nil || res.IsError {
33653519
t.Fatalf("mem_save_prompt failed: err=%v isError=%v text=%q", err, res.IsError, callResultText(t, res))
@@ -3377,6 +3531,9 @@ func TestHandleSaveAndPromptUseConfigProjectForWrites(t *testing.T) {
33773531
if err != nil || len(prompts) != 1 {
33783532
t.Fatalf("expected prompt written to config project, prompts=%d err=%v", len(prompts), err)
33793533
}
3534+
if wrong, _ := s.Search("memory saved under config project", store.SearchOptions{Project: "attempted-override", Limit: 5}); len(wrong) != 0 {
3535+
t.Fatal("explicit project choice must not override non-ambiguous/config project")
3536+
}
33803537
}
33813538

33823539
// TestResolveWriteProject_AmbiguousError: assert errors.Is(err, ErrAmbiguousProject)

internal/project/detect.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,12 @@ const (
3333
SourceDirBasename = "dir_basename" // fallback: directory basename
3434
SourceAmbiguous = "ambiguous" // cwd contains multiple git repos (Case 4)
3535
SourceExplicitOverride = "explicit_override" // JR2-2: caller explicitly supplied a project name
36-
SourceRequestBody = "request_body" // REQ-414: project came from the request body (server-side, no filesystem path)
37-
SourceConfig = "config" // derived from .engram/config.json project_name
36+
// SourceUserSelectedAfterAmbiguousProject means an MCP write initially hit
37+
// ErrAmbiguousProject and the caller provided an explicit user-selected
38+
// project from the ambiguity result's available_projects list.
39+
SourceUserSelectedAfterAmbiguousProject = "user_selected_after_ambiguous_project"
40+
SourceRequestBody = "request_body" // REQ-414: project came from the request body (server-side, no filesystem path)
41+
SourceConfig = "config" // derived from .engram/config.json project_name
3842
)
3943

4044
// noiseSet lists directory names that are skipped during child-repo scanning.

0 commit comments

Comments
 (0)