propose valid inputs where errors know them

Where the valid set behind a failed input is knowable, the error names
it instead of leaving the agent to guess: fuzzy did_you_mean proposals
for file paths (edit distance with a similarity threshold plus a
path-suffix fast path), configured_labels enumerated on a vote 400,
and a hint naming the refreshing tool for stale reply anchors.

Two misleading successes join the error contract: get_file_diff on a
path absent from the revision (Gerrit answers 200 with an empty diff)
errors with proposals, and post_comments refuses new comments on files
outside the current revision, which Gerrit accepts silently and no
reviewer ever sees; replies stay exempt since their thread may anchor
to an older patch set's file. search_changes validates start and empty
queries before the round trip (empty stays legal under project
scoping) and scoped results carry a scope attribute.

Enrichment runs on the failure path only, except the post_comments
pre-flight: one file-list fetch when new file comments are present.

ADR 1.3 amended: recovery affordances join the published contract.

Author: xobotyi

design-docs/adr/1.3-llmxml-output.md

@@ -23,3 +23,11 @@ server middleware rather than per tool, so failure output never forces a parsing
 untouched (llmxml is the model-facing layer, not the transport signal), and protocol-level failures such as an unknown
 tool name remain plain JSON-RPC errors. The error shape is part of the same published contract as success renders and
 is pinned by golden files.
+
+**Amended 2026-07-11 — errors carry a recovery path where one is knowable.** When the valid set behind a failed input
+is known, the error names it: fuzzy `did_you_mean` proposals for file paths (large legible sets), full enumeration for
+small sets (configured labels, patch-set ranges), and a `hint` field naming the tool that refreshes stale context
+(reply anchors). Proposals beyond a similarity threshold are dropped — a far-fetched proposal invites a confident
+wrong retry. The same stance covers misleading successes: an empty diff for a path absent from the revision becomes an
+error, and scoped query results name the active `scope` so an empty page reads as "possibly out of scope", not "does
+not exist". Enrichment fetches run on the failure path only.

internal/tools/error-affordances_test.go

@@ -0,0 +1,181 @@
+package tools_test
+
+import (
+	"net/http"
+	"strings"
+	"testing"
+
+	"github.com/modelcontextprotocol/go-sdk/mcp"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"dev.gaijin.team/go/go-gerrit-mcp/internal/config"
+)
+
+// Errors and empty results must carry what the agent needs to recover:
+// proposals when the valid set is known, the active scope when it silently
+// narrows a query, and the real patch-set range when a revision guess misses.
+
+func Test_SearchChanges_Boundaries(t *testing.T) {
+	t.Parallel()
+
+	t.Run("negative start refused before any request", func(t *testing.T) {
+		t.Parallel()
+
+		requested := false
+
+		cs := fakeGerritFlag(t, ")]}'\n[]", &requested)
+
+		res, err := cs.CallTool(t.Context(), &mcp.CallToolParams{
+			Name:      "search_changes",
+			Arguments: map[string]any{"query": "status:open", "start": -2},
+		})
+		require.NoError(t, err)
+		require.True(t, res.IsError)
+		assert.Contains(t, errorText(t, res), "start must be zero or positive")
+		assert.False(t, requested, "no query may leave the process")
+	})
+
+	t.Run("empty query without scope refused with a starting point", func(t *testing.T) {
+		t.Parallel()
+
+		requested := false
+
+		cs := fakeGerritFlag(t, ")]}'\n[]", &requested)
+
+		res, err := cs.CallTool(t.Context(), &mcp.CallToolParams{
+			Name:      "search_changes",
+			Arguments: map[string]any{"query": "  "},
+		})
+		require.NoError(t, err)
+		require.True(t, res.IsError)
+		assert.Contains(t, errorText(t, res), "status:open")
+		assert.False(t, requested, "no query may leave the process")
+	})
+
+	t.Run("empty query with scope browses the scope", func(t *testing.T) {
+		t.Parallel()
+
+		cs, lastURL := scopedGerrit(t, ")]}'\n[]")
+
+		out := callTool(t, cs, "search_changes", map[string]any{"query": ""})
+
+		assert.Equal(t, `<changes query="" scope="core" start="0" count="0" more="false"/>`, out)
+		assert.Contains(t, *lastURL, "project:core", "scope clause must reach the wire")
+	})
+
+	t.Run("scoped result names the scope", func(t *testing.T) {
+		t.Parallel()
+
+		cs, _ := scopedGerrit(t, ")]}'\n[]")
+
+		out := callTool(t, cs, "search_changes", map[string]any{"query": "project:elsewhere"})
+
+		assert.Equal(t,
+			`<changes query="project:elsewhere" scope="core" start="0" count="0" more="false"/>`,
+			out,
+			"the scope attribute is what tells the agent an empty page may mean out-of-scope",
+		)
+	})
+}
+
+func Test_GetFileDiff_UnknownFile(t *testing.T) {
+	t.Parallel()
+
+	const emptyDiffJSON = ")]}'\n" + `{"change_type":"MODIFIED","content":[]}`
+
+	cs := session(t, func(w http.ResponseWriter, r *http.Request) {
+		switch {
+		case r.URL.Path == "/a/accounts/self":
+			_, _ = w.Write([]byte(selfJSON))
+		case strings.HasSuffix(r.URL.Path, "/diff"):
+			_, _ = w.Write([]byte(emptyDiffJSON))
+		case strings.HasSuffix(r.URL.Path, "/files/"):
+			_, _ = w.Write([]byte(revisionFilesJSON))
+		default:
+			_, _ = w.Write([]byte(ownChangeJSON))
+		}
+	})
+
+	res, err := cs.CallTool(t.Context(), &mcp.CallToolParams{
+		Name:      "get_file_diff",
+		Arguments: map[string]any{"change": "123", "file": "core/scaner.go"},
+	})
+	require.NoError(t, err)
+	require.True(t, res.IsError, "Gerrit's empty diff for an absent file must not render as success")
+
+	golden(t, "error-diff-unknown-file", errorText(t, res))
+}
+
+func Test_ListChangeFiles_RevisionNotFound(t *testing.T) {
+	t.Parallel()
+
+	const allRevisionsJSON = ")]}'\n" + `{"_number":123,"project":"core",` +
+		`"current_revision":"sha3",` +
+		`"revisions":{"sha1":{"_number":1},"sha2":{"_number":2},"sha3":{"_number":3}}}`
+
+	cs := session(t, func(w http.ResponseWriter, r *http.Request) {
+		switch {
+		case r.URL.Path == "/a/accounts/self":
+			_, _ = w.Write([]byte(selfJSON))
+		case strings.Contains(r.URL.Path, "/revisions/99/"):
+			w.WriteHeader(http.StatusNotFound)
+
+			_, _ = w.Write([]byte("Not found: 99"))
+
+		default:
+			_, _ = w.Write([]byte(allRevisionsJSON))
+		}
+	})
+
+	res, err := cs.CallTool(t.Context(), &mcp.CallToolParams{
+		Name:      "list_change_files",
+		Arguments: map[string]any{"change": "123", "revision": "99"},
+	})
+	require.NoError(t, err)
+	require.True(t, res.IsError)
+
+	text := errorText(t, res)
+	assert.Contains(t, text, "Not found: 99")
+	assert.Contains(t, text, "valid_patch_sets=1-3")
+	assert.Contains(t, text, "current_patch_set=3")
+}
+
+// fakeGerritFlag is fakeGerrit with a flag reporting whether any API request
+// beyond the startup self-check reached the fake.
+func fakeGerritFlag(t *testing.T, fixture string, requested *bool) *mcp.ClientSession {
+	t.Helper()
+
+	return session(t, func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/a/accounts/self" {
+			_, _ = w.Write([]byte(selfJSON))
+
+			return
+		}
+
+		*requested = true
+
+		_, _ = w.Write([]byte(fixture))
+	})
+}
+
+// scopedGerrit is fakeGerrit with the project allowlist set to "core".
+func scopedGerrit(t *testing.T, fixture string) (*mcp.ClientSession, *string) {
+	t.Helper()
+
+	lastURL := new(string)
+
+	cs := session(t, func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/a/accounts/self" {
+			_, _ = w.Write([]byte(selfJSON))
+
+			return
+		}
+
+		*lastURL = r.URL.String()
+
+		_, _ = w.Write([]byte(fixture))
+	}, func(cfg *config.Config) { cfg.Projects = []string{"core"} })
+
+	return cs, lastURL
+}

internal/tools/get-change_test.go

@@ -46,13 +46,13 @@ const changeJSON = ")]}'\n" + `{
 
 // session spins a fake Gerrit plus an in-memory MCP server/client pair with
 // the read-group tool registered, and returns the connected client session.
-func session(t *testing.T, gerritHandler http.HandlerFunc) *mcp.ClientSession {
+func session(t *testing.T, gerritHandler http.HandlerFunc, mutate ...func(*config.Config)) *mcp.ClientSession {
 	t.Helper()
 
 	srv := httptest.NewServer(gerritHandler)
 	t.Cleanup(srv.Close)
 
-	client, err := gerritclient.New(t.Context(), &config.Config{
+	cfg := &config.Config{
 		GerritURL:           srv.URL,
 		Username:            "bot",
 		Token:               "s3cret",
@@ -61,7 +61,12 @@ func session(t *testing.T, gerritHandler http.HandlerFunc) *mcp.ClientSession {
 		ExcludeTools:        nil,
 		Projects:            nil,
 		AllowForeignChanges: false,
-	})
+	}
+	for _, m := range mutate {
+		m(cfg)
+	}
+
+	client, err := gerritclient.New(t.Context(), cfg)
 	require.NoError(t, err)
 
 	mcpServer := mcp.NewServer(&mcp.Implementation{Name: "test", Version: "0"}, nil)

internal/tools/get-file-diff.go

@@ -5,13 +5,17 @@ import (
 	"fmt"
 	"strings"
 
+	"dev.gaijin.team/go/golib/e"
+	"dev.gaijin.team/go/golib/fields"
 	gerrit "github.com/andygrunwald/go-gerrit"
 	"github.com/modelcontextprotocol/go-sdk/mcp"
 
 	"dev.gaijin.team/go/go-gerrit-mcp/internal/gerritclient"
 	"dev.gaijin.team/go/go-gerrit-mcp/internal/llmxml"
 )
 
+var errFileNotInRevision = e.New("file is not part of the revision")
+
 type getFileDiffInput struct {
 	Change   string `json:"change" jsonschema:"Change identifier: numeric ID, project~number, or Change-Id"`
 	File     string `json:"file" jsonschema:"File path within the change; /COMMIT_MSG addresses the commit message"`
@@ -32,12 +36,52 @@ func getFileDiff(c *gerritclient.Client) Tool {
 					return nil, nil, err
 				}
 
+				// Gerrit answers 200 with an empty diff for paths absent from
+				// the revision; distinguish "no such file" from a legitimately
+				// empty diff before rendering a misleading success.
+				if !diff.Binary && len(diff.Content) == 0 {
+					if err := checkDiffFile(ctx, c, in); err != nil {
+						return nil, nil, err
+					}
+				}
+
 				return textResult(renderDiff(in, diff)), nil, nil
 			})
 		},
 	}
 }
 
+// checkDiffFile verifies the requested file exists in the revision, turning
+// Gerrit's ambiguous empty diff into an error that carries the closest
+// actual paths. Best-effort: an unverifiable file list keeps the empty diff.
+func checkDiffFile(ctx context.Context, c *gerritclient.Client, in getFileDiffInput) error {
+	files, err := c.ListFiles(ctx, in.Change, in.Revision)
+	if err != nil {
+		return nil //nolint:nilerr // verification is best-effort, the empty diff render stands
+	}
+
+	if _, ok := files[in.File]; ok {
+		return nil
+	}
+
+	res := errFileNotInRevision.WithFields(
+		fields.F("change", in.Change),
+		fields.F("revision", revisionLabel(in.Revision)),
+		fields.F("file", in.File),
+	)
+
+	paths := make([]string, 0, len(files))
+	for path := range files {
+		paths = append(paths, path)
+	}
+
+	if near := proposals(in.File, paths); len(near) > 0 {
+		return res.WithField("did_you_mean", strings.Join(near, ", "))
+	}
+
+	return res.WithField("hint", "list_change_files names the files of the revision")
+}
+
 func renderDiff(in getFileDiffInput, diff *gerrit.DiffInfo) string {
 	root := llmxml.NewElement("diff",
 		llmxml.Attr("change", in.Change),

internal/tools/post-comments.go

@@ -22,6 +22,9 @@ var (
 	errUnknownReplyTo = e.New("reply targets do not exist on the change")
 	errCommentNoText  = e.New("comment message must not be empty")
 	errCommentNoFile  = e.New("comment file must not be empty")
+	// errCommentFileUnknown guards against Gerrit's silent acceptance of
+	// comments on paths outside the change, which no reviewer ever sees.
+	errCommentFileUnknown = e.New("comment file is not part of the current revision")
 )
 
 type inlineComment struct {
@@ -48,10 +51,11 @@ func postComments(c *gerritclient.Client) Tool {
 			mcp.AddTool(s, &mcp.Tool{
 				Name: NamePostComments,
 				Description: "Post a review to a Gerrit change in one call: optional top-level message " +
-					"plus inline, range, file-level, and reply comments. Replies anchor to comment ids " +
-					"from get_change_comments; setting resolved on a reply toggles the thread state. " +
-					"Refused on changes not owned by the authenticated account unless the operator " +
-					"disabled the own-changes restriction.",
+					"plus inline, range, file-level, and reply comments. New comments must name a file " +
+					"of the current revision (or /COMMIT_MSG, /PATCHSET_LEVEL); replies anchor to " +
+					"comment ids from get_change_comments, and setting resolved on a reply toggles the " +
+					"thread state. Refused on changes not owned by the authenticated account unless " +
+					"the operator disabled the own-changes restriction.",
 			}, func(ctx context.Context, _ *mcp.CallToolRequest, in postCommentsInput,
 			) (*mcp.CallToolResult, any, error) {
 				input, err := buildReviewInput(ctx, c, in)
@@ -84,6 +88,10 @@ func buildReviewInput(ctx context.Context, c *gerritclient.Client, in postCommen
 		return nil, err
 	}
 
+	if err := validateCommentFiles(ctx, c, in); err != nil {
+		return nil, err
+	}
+
 	if err := validateReplyTargets(ctx, c, in); err != nil {
 		return nil, err
 	}
@@ -201,7 +209,66 @@ func validateReplyTargets(ctx context.Context, c *gerritclient.Client, in postCo
 		return errUnknownReplyTo.WithFields(
 			fields.F("change", in.Change),
 			fields.F("ids", strings.Join(missing, ",")),
+			fields.F("hint", "comment ids may be stale; get_change_comments returns current anchors"),
+		)
+	}
+
+	return nil
+}
+
+// magicPath reports whether the path is one of Gerrit's virtual files,
+// commentable on every change.
+func magicPath(path string) bool {
+	switch path {
+	case "/COMMIT_MSG", "/PATCHSET_LEVEL", "/MERGE_LIST":
+		return true
+	default:
+		return false
+	}
+}
+
+// validateCommentFiles refuses new comments on files absent from the current
+// revision — Gerrit silently accepts them and the comment is never seen next
+// to code. Replies are exempt: their thread may anchor to a file of an older
+// patch set. Costs one file-list fetch, only when new file comments exist.
+func validateCommentFiles(ctx context.Context, c *gerritclient.Client, in postCommentsInput) error {
+	var unchecked []string
+
+	for _, comment := range in.Comments {
+		if comment.ReplyTo == "" && !magicPath(comment.File) {
+			unchecked = append(unchecked, comment.File)
+		}
+	}
+
+	if len(unchecked) == 0 {
+		return nil
+	}
+
+	files, err := c.ListFiles(ctx, in.Change, "")
+	if err != nil {
+		return err
+	}
+
+	for _, file := range unchecked {
+		if _, ok := files[file]; ok {
+			continue
+		}
+
+		res := errCommentFileUnknown.WithFields(
+			fields.F("change", in.Change),
+			fields.F("file", file),
 		)
+
+		paths := make([]string, 0, len(files))
+		for path := range files {
+			paths = append(paths, path)
+		}
+
+		if near := proposals(file, paths); len(near) > 0 {
+			return res.WithField("did_you_mean", strings.Join(near, ", "))
+		}
+
+		return res.WithField("hint", "list_change_files names the files of the current revision")
 	}
 
 	return nil