fix(mcp): reject unknown facade repo selectors

internal/mcp/diff_repo_scope_test.go

@@ -62,11 +62,13 @@ func TestDiffRepoScope(t *testing.T) {
 	require.Equal(t, repoA, root)
 	require.Equal(t, "repo-a", prefix)
 
-	// An unknown selector resolves nothing — the caller errors instead of
-	// falling back to an unrelated repo.
-	root, prefix = srv.diffRepoScope(ctx, "nope")
-	require.Empty(t, root)
-	require.Empty(t, prefix)
+	// Unknown prefix and absolute path selectors both resolve nothing — the
+	// caller errors instead of falling back to an unrelated active repo.
+	for _, selector := range []string{"nope", t.TempDir()} {
+		root, prefix = srv.diffRepoScope(ctx, selector)
+		require.Empty(t, root)
+		require.Empty(t, prefix)
+	}
 
 	// No selector + multiple tracked repos + no session binding: ambiguous.
 	root, prefix = srv.diffRepoScope(ctx, "")

internal/mcp/facade_container_validation_test.go

@@ -0,0 +1,207 @@
+package mcp
+
+import (
+	"context"
+	"encoding/json"
+	"os"
+	"path/filepath"
+	"testing"
+
+	mcpgo "github.com/mark3labs/mcp-go/mcp"
+	"github.com/stretchr/testify/require"
+)
+
+func TestFacadeRepositoryValidationRejectsUnconsumedPathsFromRealRegistry(t *testing.T) {
+	srv, _ := setupTestServer(t)
+
+	for _, test := range []struct {
+		name          string
+		facade        string
+		operation     string
+		input         map[string]any
+		wantField     string
+		wantSuggested string
+	}{
+		{
+			name: "change detect source repo path", facade: "change", operation: "detect",
+			input:     map[string]any{"source": map[string]any{"repo_path": `C:\work\other-repo`}},
+			wantField: "source.repo_path", wantSuggested: "options.repo",
+		},
+		{
+			name: "top-level repo path", facade: "change", operation: "detect",
+			input: map[string]any{"repo_path": `C:\work\other-repo`}, wantField: "repo_path",
+		},
+		{
+			name: "context repo path", facade: "change", operation: "detect",
+			input: map[string]any{"context": map[string]any{"repo_path": `C:\work\other-repo`}}, wantField: "context.repo_path",
+		},
+		{
+			name: "guard repo path", facade: "change", operation: "detect",
+			input: map[string]any{"guard": map[string]any{"repo_path": `C:\work\other-repo`}}, wantField: "guard.repo_path",
+		},
+		{
+			name: "cold pr source repo path", facade: "pr", operation: "risk",
+			input: map[string]any{"source": map[string]any{"repo_path": `C:\work\other-repo`}}, wantField: "source.repo_path",
+			wantSuggested: "arguments.repo",
+		},
+		{
+			name: "cold pr context repository", facade: "pr", operation: "list",
+			input: map[string]any{"context": map[string]any{"repository": "other-repo"}}, wantField: "context.repository",
+		},
+		{
+			name: "external write source repo path", facade: "publish_review", operation: "post",
+			input: map[string]any{"source": map[string]any{"repo_path": `C:\work\other-repo`}}, wantField: "source.repo_path",
+			wantSuggested: "arguments.repo",
+		},
+		{
+			name: "read symbols injected baseline", facade: "read", operation: "symbols",
+			input: map[string]any{"context": map[string]any{"repo_path": `C:\work\other-repo`}}, wantField: "context.repo_path",
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			spec, ok := srv.facades.operation(test.facade, test.operation)
+			require.True(t, ok, "real registry must contain %s.%s", test.facade, test.operation)
+			test.input["operation"] = test.operation
+
+			result := srv.validateFacadeInput(spec, test.input)
+			require.NotNil(t, result)
+			require.True(t, result.IsError)
+			var structured StructuredError
+			require.NoError(t, json.Unmarshal([]byte(toolResultText(result)), &structured))
+			require.Equal(t, ErrCodeInvalidArgument, structured.ErrorCode)
+			require.Equal(t, test.wantField, structured.Data["field"])
+			if test.wantSuggested != "" {
+				require.Equal(t, test.wantSuggested, structured.Data["suggested_field"])
+			}
+		})
+	}
+}
+
+func TestFacadeRepositoryValidationPreservesConsumedCompatibilityFields(t *testing.T) {
+	srv, _ := setupTestServer(t)
+
+	for _, test := range []struct {
+		name        string
+		facade      string
+		operation   string
+		container   string
+		fields      map[string]any
+		wantLowered []string
+	}{
+		{
+			name: "change source base ref and repo", facade: "change", operation: "detect", container: "source",
+			fields: map[string]any{"base_ref": "HEAD", "repo": "tracked-repo"}, wantLowered: []string{"base_ref", "repo"},
+		},
+		{
+			name: "change output repo", facade: "change", operation: "detect", container: "output",
+			fields: map[string]any{"repo": "tracked-repo"}, wantLowered: []string{"repo"},
+		},
+		{
+			name: "review source base ref", facade: "review", operation: "run", container: "source",
+			fields: map[string]any{"base_ref": "HEAD"}, wantLowered: []string{"base_ref"},
+		},
+		{
+			name: "change ranges source fields", facade: "change", operation: "ranges", container: "source",
+			fields: map[string]any{"path": "main.go", "start_line": 1, "end_line": 2}, wantLowered: []string{"path", "start_line", "end_line"},
+		},
+		{
+			name: "change contract source fields", facade: "change", operation: "contract", container: "source",
+			fields: map[string]any{"lens": "api", "risk_gate": "strict"}, wantLowered: []string{"lens", "risk_gate"},
+		},
+		{
+			name: "change simulate source keep", facade: "change", operation: "simulate", container: "source",
+			fields: map[string]any{"keep": true}, wantLowered: []string{"keep"},
+		},
+		{
+			name: "review critique prior review", facade: "review", operation: "critique", container: "source",
+			fields: map[string]any{"prior_review": "review text"}, wantLowered: []string{"prior_review"},
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			spec, ok := srv.facades.operation(test.facade, test.operation)
+			require.True(t, ok, "real registry must contain %s.%s", test.facade, test.operation)
+			input := map[string]any{"operation": test.operation, test.container: test.fields}
+
+			require.Nil(t, srv.validateFacadeInput(spec, input))
+			normalized := normalizeFacadeArguments(spec, input)
+			for _, field := range test.wantLowered {
+				_, exists := normalized[field]
+				require.True(t, exists, "%s must survive normalization", field)
+				require.True(t, srv.legacyDeclaresField(spec.Legacy, field), "%s must be consumed by %s", field, spec.Legacy)
+			}
+		})
+	}
+}
+
+func TestFacadeRepositoryValidationLeavesNonSelectorCompatibilityAlone(t *testing.T) {
+	srv, _ := setupTestServer(t)
+	spec, ok := srv.facades.operation("change", "detect")
+	require.True(t, ok)
+	require.Nil(t, srv.validateFacadeInput(spec, map[string]any{
+		"operation": "detect",
+		"source":    map[string]any{"base": "HEAD", "mystery": true},
+	}))
+}
+
+func TestFacadeRepositoryValidationRejectsInvalidCanonicalSelectors(t *testing.T) {
+	srv, _ := setupTestServer(t)
+
+	for _, test := range []struct {
+		name      string
+		facade    string
+		operation string
+		input     map[string]any
+		wantField string
+	}{
+		{
+			name: "empty common selector", facade: "change", operation: "detect",
+			input: map[string]any{"options": map[string]any{"repo": "  "}}, wantField: "options.repo",
+		},
+		{
+			name: "non-string common selector", facade: "change", operation: "detect",
+			input: map[string]any{"options": map[string]any{"repo": 42}}, wantField: "options.repo",
+		},
+		{
+			name: "empty cold selector", facade: "pr", operation: "risk",
+			input: map[string]any{"arguments": map[string]any{"repo": ""}}, wantField: "arguments.repo",
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			spec, ok := srv.facades.operation(test.facade, test.operation)
+			require.True(t, ok)
+			test.input["operation"] = test.operation
+
+			result := srv.validateFacadeInput(spec, test.input)
+			require.NotNil(t, result)
+			require.True(t, result.IsError)
+			var structured StructuredError
+			require.NoError(t, json.Unmarshal([]byte(toolResultText(result)), &structured))
+			require.Equal(t, test.wantField, structured.Data["field"])
+			require.Equal(t, "non-empty string", structured.Data["expected_type"])
+		})
+	}
+}
+
+func TestFacadeEditFileRejectsUnconsumedRepositoryBeforeWrite(t *testing.T) {
+	srv, root := setupTestServer(t)
+	target := filepath.Join(root, "main.go")
+	before, err := os.ReadFile(target)
+	require.NoError(t, err)
+
+	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",
+		"options":     map[string]any{"repo": t.TempDir()},
+	}
+	result, err := srv.handleFacade(context.Background(), "edit", req)
+	require.NoError(t, err)
+	require.True(t, result.IsError)
+
+	after, err := os.ReadFile(target)
+	require.NoError(t, err)
+	require.Equal(t, before, after)
+}

internal/mcp/facade_localization_scope_test.go

@@ -14,7 +14,11 @@ import (
 
 func TestFacadeExplorePathLowersToExplicitRepoSelector(t *testing.T) {
 	registry := newFacadeRegistry()
-	registry.capture(mcpgo.NewTool("explore", mcpgo.WithString("task", mcpgo.Required())), func(_ context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
+	registry.capture(mcpgo.NewTool(
+		"explore",
+		mcpgo.WithString("task", mcpgo.Required()),
+		mcpgo.WithString("repo"),
+	), func(_ context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
 		return mcpgo.NewToolResultJSON(req.GetArguments())
 	})
 	server := &Server{facades: registry, localization: newLocalizationTerminalState()}

internal/mcp/facade_tools.go

@@ -6,6 +6,7 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
+	"reflect"
 	"slices"
 	"sort"
 	"strings"
@@ -982,10 +983,6 @@ func (s *Server) invokeFacadeSpec(ctx context.Context, req mcpgo.CallToolRequest
 			Data:      map[string]any{"facade": spec.Facade, "operation": spec.Operation, "legacy_tool": spec.Legacy},
 		}), nil
 	}
-	if invalid := s.validateFacadeInput(spec, req.GetArguments()); invalid != nil {
-		outcome = facadeOutcomeInvalidArgument
-		return invalid, nil
-	}
 	normalized := normalizeFacadeArguments(spec, req.GetArguments())
 	if targetErr := normalizeFacadeChangeTargets(spec, req.GetArguments(), normalized); targetErr != nil {
 		outcome = facadeOutcomeInvalidArgument
@@ -1457,7 +1454,7 @@ func (s *Server) facadeAcceptedTopLevelFields(spec facadeOperationSpec) []string
 }
 
 func (s *Server) validateFacadeInput(spec facadeOperationSpec, input map[string]any) *mcpgo.CallToolResult {
-	for _, field := range []string{"arguments", "options", "source", "context", "guard", "output"} {
+	for _, field := range facadeContainerKeys {
 		value, present := input[field]
 		if !present || value == nil {
 			continue
@@ -1489,6 +1486,9 @@ func (s *Server) validateFacadeInput(spec facadeOperationSpec, input map[string]
 			})
 		}
 	}
+	if invalid := s.validateFacadeRepositoryFields(spec, input); invalid != nil {
+		return invalid
+	}
 	for _, field := range []string{"target", "to"} {
 		if raw, present := input[field]; present && raw != nil {
 			if invalid := validateFacadeSelector(field, raw); invalid != nil {
@@ -1528,6 +1528,110 @@ func (s *Server) validateFacadeInput(spec facadeOperationSpec, input map[string]
 	return nil
 }
 
+// validateFacadeRepositoryFields rejects repository-selector spellings only
+// when the selected legacy handler cannot consume their normalized form. This
+// preserves working compatibility aliases while closing every top-level and
+// nested-container path that would otherwise silently target the active repo.
+func (s *Server) validateFacadeRepositoryFields(spec facadeOperationSpec, input map[string]any) *mcpgo.CallToolResult {
+	locations := append([]string{""}, facadeContainerKeys...)
+	for _, containerName := range locations {
+		fields := input
+		if containerName != "" {
+			var ok bool
+			fields, ok = input[containerName].(map[string]any)
+			if !ok {
+				continue
+			}
+		}
+		for _, field := range sortedFacadeMapKeys(fields) {
+			if !facadeRepositorySelectorLike(field) {
+				continue
+			}
+			path := field
+			if containerName != "" {
+				path = containerName + "." + field
+			}
+			canonicalPath := s.facadePublicRepositoryField(spec)
+			if path == canonicalPath {
+				value, ok := fields[field].(string)
+				if !ok || strings.TrimSpace(value) == "" {
+					return NewStructuredErrorResult(StructuredError{
+						ErrorCode: ErrCodeInvalidArgument,
+						Message:   fmt.Sprintf("%s must be a non-empty string", path),
+						Data: map[string]any{
+							"field": path, "expected_type": "non-empty string",
+						},
+					})
+				}
+			}
+			if s.facadeFieldConsumed(spec, containerName, field, fields[field]) {
+				continue
+			}
+			data := map[string]any{"field": path}
+			if containerName != "" {
+				data["container"] = containerName
+			}
+			message := fmt.Sprintf("unknown field %q", path)
+			if canonicalPath != "" {
+				data["suggested_field"] = canonicalPath
+				message += fmt.Sprintf("; use %s to select a repository", canonicalPath)
+			}
+			return NewStructuredErrorResult(StructuredError{
+				ErrorCode: ErrCodeInvalidArgument,
+				Message:   message,
+				Data:      data,
+			})
+		}
+	}
+	return nil
+}
+
+func (s *Server) facadeFieldConsumed(spec facadeOperationSpec, containerName, field string, value any) bool {
+	baseline := normalizeFacadeArguments(spec, map[string]any{"operation": spec.Operation})
+	probe := map[string]any{"operation": spec.Operation}
+	if containerName == "" {
+		probe[field] = value
+	} else {
+		probe[containerName] = map[string]any{field: value}
+	}
+	for lowered, candidateValue := range normalizeFacadeArguments(spec, probe) {
+		if _, fixed := spec.Fixed[lowered]; fixed || !s.legacyDeclaresField(spec.Legacy, lowered) {
+			continue
+		}
+		baselineValue, existed := baseline[lowered]
+		if !existed || !reflect.DeepEqual(baselineValue, candidateValue) {
+			return true
+		}
+	}
+	return false
+}
+
+func (s *Server) facadePublicRepositoryField(spec facadeOperationSpec) string {
+	capability := s.facadeCapability(spec, true)
+	schema, _ := capability["input_schema"].(map[string]any)
+	properties, _ := schema["properties"].(map[string]any)
+	if _, ok := properties["repo"]; ok {
+		return "repo"
+	}
+	for _, container := range facadeContainerKeys {
+		containerSchema, _ := properties[container].(map[string]any)
+		containerProperties, _ := containerSchema["properties"].(map[string]any)
+		if _, ok := containerProperties["repo"]; ok {
+			return container + ".repo"
+		}
+	}
+	return ""
+}
+
+func facadeRepositorySelectorLike(field string) bool {
+	switch strings.ToLower(strings.TrimSpace(field)) {
+	case "repo", "repo_path", "repository", "repository_path":
+		return true
+	default:
+		return false
+	}
+}
+
 func validateFacadeSelector(field string, raw any) *mcpgo.CallToolResult {
 	target, ok := raw.(map[string]any)
 	if !ok {