fix(mcp): close the remaining doors to a silent wrong-repository target

docs/mcp-facade-v1.md

@@ -227,7 +227,13 @@ High-frequency fields are direct and typed. In particular, `edit` advertises `ma
 
 Cold domain tools accept an `arguments` object; common tools may additionally accept `options`, `source`, or `context`. Repository/project/scope fields use the operation schema returned by `capabilities` and are passed through to the handler. `output` is a stable open object for response shaping such as `format`, `max_bytes`, `limit`, `cursor`, and `fields`; adding another response control does not change the outer tool schema. Cursors remain opaque.
 
-A repository-selector field — `repo`, `repo_path`, `repository`, or `repository_path`, in any letter case, at the top level or in any container — MUST be refused with `invalid_argument` when the selected operation cannot consume it. Silently dropping it answers about the active repository while the caller believes another one was addressed, which is a wrong answer the caller cannot detect, and on a write operation it is a write to the wrong repository. Most operations accept no repository selector at all and run against the active project; their refusal carries `data.reason = "no_repository_selector"` and names `workspace_admin.set_active_project` as the way to change scope. An operation that does publish one names it in `data.suggested_field` — `options.repo` for the common domains, `arguments.repo` for cold domains. `capabilities(...detail="schema")` is authoritative for which selector, if any, an operation accepts.
+A field that states **which repository or workspace** an operation should act on MUST be refused with `invalid_argument` when the selected operation cannot consume it. Silently dropping it answers about the active repository while the caller believes another one was addressed, which is a wrong answer the caller cannot detect, and on a write operation it is a write to the wrong repository.
+
+The rule applies at the top level and in every container, in any letter case, and covers the spellings a caller may reasonably invent as well as the published one: `repo`, `repo_path`, `repository`, `repository_path`, `repo_root`, `repository_root`, `repoPath`, `repo-path`, `repo_dir`, `root`, `cwd`, `dir`, `worktree`, `work_tree`, `base_repo`, `workspace`, `project`. Inventing a name does not make the intent less clear, so it must not make the failure quieter. `path` and `scope` are deliberately excluded: on this surface they name a file or a working-tree scope far more often than a repository.
+
+Refusal is decided per operation by whether the field reaches a reader, not by the name alone — an operation that genuinely consumes `workspace` or `root` still receives it. Most operations consume no repository selector at all and run against the active project; their refusal carries `data.reason = "no_repository_selector"` and names `workspace_admin.set_active_project` as the way to change scope. An operation that does publish one names it in `data.suggested_field` — `options.repo` for the common domains, `arguments.repo` for cold domains. `capabilities(...detail="schema")` is authoritative for which selector, if any, an operation accepts.
+
+Fields outside this class that an operation does not consume are still forwarded and ignored. Closing that wider gap requires enumerating every server-side reader — the handler, the response layer, and facade middleware each read from the normalized arguments — and an incomplete enumeration would refuse working calls, so the guarantee here is deliberately limited to fields that name a target.
 
 ### 8.4 Response compatibility and metadata
 

internal/mcp/facade_target_selector_test.go

@@ -0,0 +1,168 @@
+package mcp
+
+import (
+	"context"
+	"encoding/json"
+	"os"
+	"path/filepath"
+	"testing"
+
+	mcpgo "github.com/mark3labs/mcp-go/mcp"
+	"github.com/stretchr/testify/require"
+)
+
+// A caller who invents a spelling for "operate on that repository" has stated
+// the same intent as one who writes `repo`. Answering about the active
+// repository instead is wrong for every spelling, so each must fail closed —
+// and on a write, before anything is written.
+func TestFacadeRefusesInventedRepositorySelectorsBeforeWrite(t *testing.T) {
+	srv, root := setupTestServer(t)
+	target := filepath.Join(root, "main.go")
+	before, err := os.ReadFile(target)
+	require.NoError(t, err)
+
+	elsewhere := t.TempDir()
+	for _, container := range []string{"options", "arguments", "source", "context", "guard", "output"} {
+		for _, spelling := range []string{
+			"repo", "repo_path", "repository", "repository_path",
+			"repo_root", "repository_root", "repoPath", "repo_dir",
+			"root", "cwd", "dir", "worktree", "base_repo",
+			"workspace", "project",
+		} {
+			t.Run(container+"."+spelling, func(t *testing.T) {
+				req := mcpgo.CallToolRequest{}
+				req.Params.Name = "edit"
+				req.Params.Arguments = map[string]any{
+					"operation":   "file",
+					"target":      map[string]any{"file": "main.go"},
+					"match":       string(before),
+					"replacement": string(before) + "\n// must not be written\n",
+					container:     map[string]any{spelling: elsewhere},
+				}
+				result, err := srv.handleFacade(context.Background(), "edit", req)
+				require.NoError(t, err)
+				require.True(t, result.IsError, "edit.file must refuse %s.%s", container, spelling)
+
+				after, err := os.ReadFile(target)
+				require.NoError(t, err)
+				require.Equal(t, before, after, "the active repository must be untouched")
+			})
+		}
+	}
+}
+
+// The refusal must not cost a working capability: an operation that reads the
+// selector still gets it, whichever container carried it.
+func TestFacadeKeepsConsumedRepositorySelectors(t *testing.T) {
+	srv, _ := setupTestServer(t)
+	spec, ok := srv.facades.operation("change", "detect")
+	require.True(t, ok)
+
+	for _, container := range []string{"options", "source", "output", "arguments"} {
+		t.Run(container, func(t *testing.T) {
+			input := map[string]any{
+				"operation": "detect",
+				container:   map[string]any{"repo": "tracked-repo"},
+			}
+			require.Nil(t, srv.validateFacadeInput(spec, input))
+			require.Equal(t, "tracked-repo", normalizeFacadeArguments(spec, input)["repo"])
+		})
+	}
+}
+
+// `path` and `scope` name a file or a working-tree scope far more often than a
+// repository on this surface. Promoting them to target selectors would refuse
+// working calls, so the exclusion is deliberate and pinned here.
+func TestFacadeSelectorClassExcludesOverloadedNames(t *testing.T) {
+	for _, field := range []string{"path", "scope", "file", "query", "symbol", "target"} {
+		require.False(t, facadeRepositorySelectorLike(field),
+			"%q addresses something other than a repository on this surface", field)
+	}
+	for _, field := range []string{"REPO", "Repo_Path", " repository "} {
+		require.True(t, facadeRepositorySelectorLike(field),
+			"%q is a repository selector whatever its casing or padding", field)
+	}
+}
+
+// The friendly match/replacement pair belongs to the edit facade. Translating
+// it everywhere silently dropped remember.edit_memory's replacement into a
+// field edit_memory does not declare, so the memory was edited with no
+// replacement text and the call still reported success.
+func TestFacadeEditMemoryReceivesItsOwnReplacementVocabulary(t *testing.T) {
+	srv, _ := setupTestServer(t)
+	spec, ok := srv.facades.operation("remember", "edit_memory")
+	require.True(t, ok)
+	require.True(t, srv.legacyDeclaresField(spec.Legacy, "replacement"),
+		"edit_memory speaks replacement natively; the premise of this test is that it must receive it")
+
+	captured, ok := srv.facades.legacy(spec.Legacy)
+	require.True(t, ok)
+	var got map[string]any
+	srv.facades.capture(captured.tool, func(_ context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
+		got = req.GetArguments()
+		return mcpgo.NewToolResultText("{}"), nil
+	})
+
+	req := mcpgo.CallToolRequest{}
+	req.Params.Name = "remember"
+	req.Params.Arguments = map[string]any{
+		"operation": "edit_memory",
+		"arguments": map[string]any{"id": "mem-1", "pattern": "old", "replacement": "new"},
+	}
+	result, err := srv.handleFacade(context.Background(), "remember", req)
+	require.NoError(t, err)
+	require.False(t, result.IsError, "%s", toolResultText(result))
+
+	require.Equal(t, "new", got["replacement"], "the handler must receive the caller's replacement")
+	require.NotContains(t, got, "new_string", "the edit facade's vocabulary must not leak here")
+}
+
+// The edit facade still translates its own published vocabulary.
+func TestFacadeEditAliasTranslationMatchesPublishedVocabulary(t *testing.T) {
+	for _, facade := range facadeToolNames() {
+		properties := facadeToolDefinition(facade).InputSchema.Properties
+		_, publishesMatch := properties["match"]
+		_, publishesReplacement := properties["replacement"]
+		require.Equal(t, publishesMatch && publishesReplacement, facadeTranslatesEditAliases(facade),
+			"%s translates match/replacement only if it publishes them", facade)
+	}
+
+	srv, _ := setupTestServer(t)
+	spec, ok := srv.facades.operation("edit", "file")
+	require.True(t, ok)
+	lowered := normalizeFacadeArguments(spec, map[string]any{
+		"operation": "file", "match": "old", "replacement": "new",
+	})
+	require.Equal(t, "old", lowered["old_string"])
+	require.Equal(t, "new", lowered["new_string"])
+}
+
+// A refusal names the operation and, where one exists, the selector to use.
+func TestFacadeInventedSelectorRefusalIsActionable(t *testing.T) {
+	srv, _ := setupTestServer(t)
+
+	detect, ok := srv.facades.operation("change", "detect")
+	require.True(t, ok)
+	refused := srv.validateFacadeInput(detect, map[string]any{
+		"operation": "detect",
+		"options":   map[string]any{"repo_root": "/work/other"},
+	})
+	require.NotNil(t, refused)
+	var suggested StructuredError
+	require.NoError(t, json.Unmarshal([]byte(toolResultText(refused)), &suggested))
+	require.Equal(t, "options.repo_root", suggested.Data["field"])
+	require.Equal(t, "options.repo", suggested.Data["suggested_field"])
+
+	file, ok := srv.facades.operation("edit", "file")
+	require.True(t, ok)
+	refusedWrite := srv.validateFacadeInput(file, map[string]any{
+		"operation": "file",
+		"options":   map[string]any{"workspace": "other"},
+	})
+	require.NotNil(t, refusedWrite)
+	var explained StructuredError
+	require.NoError(t, json.Unmarshal([]byte(toolResultText(refusedWrite)), &explained))
+	require.Equal(t, "options.workspace", explained.Data["field"])
+	require.Equal(t, "no_repository_selector", explained.Data["reason"])
+	require.Contains(t, explained.Message, "workspace_admin.set_active_project")
+}