fix(mcp): stdio's item patch gives line_hint remote's rule through one shared helper (BUG-4082) (#2315)

* fix(mcp): stdio's item patch gives line_hint remote's rule through one shared helper (BUG-4082)

stdio's pad_item patch was passThrough, so edits reached the CLI unread:
its JSON decode into bodypatch.Edit.LineHint (an int) refused a string
hint that remote coerces ("5"), and took a JSON number of 2^53 or more,
which mcp-go's float64 decode may already have rounded, as a line number.
Remote has validated each edit's line_hint since BUG-4065, inline in
mapItemPatch.

That loop is now editsWithLineHints, called unchanged by mapItemPatch and
by a new stdio action, actionItemPatch. On stdio:
- a valid hint is forwarded as its int64;
- a refusal is validation_failed with remote's message, naming the edit
  ("(in edits[1])");
- null is absent, and a non-object edit keeps its path.
Neither transport modifies the caller's map.

Tests (lead ruling 86931478): one table, bug4082Cases, runs every input
through both transports and requires the same forwarded edits or the same
refusal (C1). Remote's BUG-4065 rows stay green unchanged. A row on each
transport pins the caller's map (C2). Red on main: stdio forwards "010",
1.5, 2^53, true and {} raw while every remote half passes.

ToolSurfaceVersion 0.105 -> 0.106 (C3), with the changelog in version.go
and docs/mcp.md; the README and the instructions.md heading follow. The
wire goldens follow in the next commit.

Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV

* test(mcp): stdio's line_hint refusal checked on its own; actionItemPatch's input pinned directly (BUG-4082)

Mutants on 928bc363f: H2 (the helper drops the edit index) turned only
remote red, because stdio was compared with remote's message, which
changed alike. Stdio now also checks the expected text. S2 (the action
writes the new edits into its input) survived: the fan-out handler hands
actions a private copy, so the handler-level C2 row cannot see it. A row
calls actionItemPatch directly.

Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV

* test(mcp): wire goldens follow ToolSurfaceVersion 0.106 (BUG-4082)

Version string only: the capability version and the instructions heading
in the two initialize goldens. With 0.105 and 0.106 normalised, the old and
new lines are identical.

Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV

* docs(mcp): name the layer that rounds a 2^53 line_hint (BUG-4082 r1)

r1 hunter Low, ruled FIX (comment and docs only): the actionItemPatch
comment, the v0.106 changelog in version.go and docs/mcp.md said "the
decoder may already have rounded" a hint of 2^53 or more, which read as
the CLI's JSON decode, which is exact. They now name mcp-go's float64
decode of the request.

Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV

README.md

@@ -402,7 +402,7 @@ directory for `claude-code`, and an `[mcp_servers.pad]` table in
 project-scoped, it's install-on-request only — `--all` and `pad mcp status` cover
 the per-user clients (including Codex) and skip it.
 
-**Tool catalog (v0.105)** — eleven resource × action tools plus `pad_set_workspace`. Undeclared input keys are rejected with a structured error rather than silently dropped, and field values are typed against the collection schema on the server:
+**Tool catalog (v0.106)** — eleven resource × action tools plus `pad_set_workspace`. Undeclared input keys are rejected with a structured error rather than silently dropped, and field values are typed against the collection schema on the server:
 
 | Tool | Actions |
 |---|---|
@@ -427,7 +427,7 @@ Plus resources at `pad://workspaces`, `pad://workspace/{ws}/dashboard`,
 and `pad://_meta/version`.
 
 **Stability contract.** The catalog is versioned
-(`tool_surface_version: "0.105"`) and advertised in the initialize handshake and at
+(`tool_surface_version: "0.106"`) and advertised in the initialize handshake and at
 `pad://_meta/version`, so external agents can pin against it; the per-version
 changelog lives in [`internal/mcp/version.go`](internal/mcp/version.go). Errors
 come back as structured envelopes (`{error: {code, message, hint, ...}}`) from a

internal/mcp/bug4082_line_hint_test.go

@@ -0,0 +1,178 @@
+package mcp
+
+import (
+	"context"
+	"encoding/json"
+	"reflect"
+	"strings"
+	"testing"
+)
+
+// BUG-4082: pad_item patch's edits[].line_hint gets one rule on both
+// transports. Remote's mapItemPatch had it inline (BUG-4065); stdio forwarded
+// edits to the CLI unread, so a string hint was refused there where remote
+// coerced it, and 2^53 or more reached bodypatch as a line number. Both now
+// call editsWithLineHints. Lead ruling 86931478: C1, one table drives both
+// transports, so a change to the helper turns both red and stdio == remote is
+// pinned, not asserted; C2, neither transport modifies the caller's map.
+
+// bug4082Cases is the shared table. edits is the JSON the caller sends; want
+// is the edits JSON each transport must forward (after a decode and re-encode,
+// so key order and number spelling do not matter); refuse, when set, is the
+// refusal both must give instead, with nothing forwarded.
+var bug4082Cases = []struct {
+	name, edits, want, refuse string
+}{
+	{"string hint coerced", `[{"old":"a","new":"b","line_hint":"5"}]`, `[{"line_hint":5,"new":"b","old":"a"}]`, ""},
+	{"number hint kept", `[{"old":"a","new":"b","line_hint":5}]`, `[{"line_hint":5,"new":"b","old":"a"}]`, ""},
+	{"null hint absent", `[{"old":"a","new":"b","line_hint":null}]`, `[{"line_hint":null,"new":"b","old":"a"}]`, ""},
+	{"no hint", `[{"old":"a","new":"b"}]`, `[{"new":"b","old":"a"}]`, ""},
+	{"largest safe hint", `[{"old":"a","new":"b","line_hint":9007199254740991}]`, `[{"line_hint":9007199254740991,"new":"b","old":"a"}]`, ""},
+	{"non-object edit left alone", `["x",{"old":"a","new":"b","line_hint":"2"}]`, `["x",{"line_hint":2,"new":"b","old":"a"}]`, ""},
+	{"leading zero", `[{"old":"a","new":"b","line_hint":"010"}]`, "", `line_hint must be a whole number, got the string \"010\" (in edits[0])`},
+	{"fraction", `[{"old":"a","new":"b","line_hint":1.5}]`, "", "line_hint must be a whole number, got 1.5 (in edits[0])"},
+	{"2^53", `[{"old":"a","new":"b","line_hint":9007199254740992}]`, "", "line_hint is beyond the exact integer range of a JSON number, got 9.007199254740992e+15 (in edits[0])"},
+	{"bool", `[{"old":"a","new":"b","line_hint":true}]`, "", "line_hint must be a number, got a boolean (in edits[0])"},
+	{"object", `[{"old":"a","new":"b","line_hint":{}}]`, "", "line_hint must be a number, got an object (in edits[0])"},
+	{"second edit names its index", `[{"old":"a","new":"b","line_hint":1},{"old":"c","new":"d","line_hint":"0x10"}]`, "", "(in edits[1])"},
+}
+
+func canonicalJSON(t *testing.T, s string) string {
+	t.Helper()
+	var v any
+	if err := json.Unmarshal([]byte(s), &v); err != nil {
+		t.Fatalf("decode %s: %v", s, err)
+	}
+	b, err := json.Marshal(v)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return string(b)
+}
+
+func bug4082Input(t *testing.T, edits string) map[string]any {
+	t.Helper()
+	var in map[string]any
+	if err := json.Unmarshal([]byte(`{"workspace":"docapp","ref":"TASK-1","edits":`+edits+`}`), &in); err != nil {
+		t.Fatal(err)
+	}
+	return in
+}
+
+// remoteLineHints runs the remote mapper and returns the forwarded edits JSON
+// or the refusal text.
+func remoteLineHints(t *testing.T, edits string) (forwarded, refusal string) {
+	t.Helper()
+	_, _, body, err := mapItemPatch(bug4082Input(t, edits))
+	if err != nil {
+		return "", err.Error()
+	}
+	var b map[string]json.RawMessage
+	if err := json.Unmarshal(body, &b); err != nil {
+		t.Fatalf("remote body %s: %v", body, err)
+	}
+	return canonicalJSON(t, string(b["edits"])), ""
+}
+
+// stdioLineHints runs the stdio fan-out handler and returns the --edits JSON
+// forwarded to the CLI or the refusal text.
+func stdioLineHints(t *testing.T, edits string) (forwarded, refusal string) {
+	t.Helper()
+	in := bug4082Input(t, edits)
+	in["action"] = "patch"
+	d := &fakeDispatcher{}
+	env := ActionEnv{Doc: liveCmdhelpDoc(t), Workspace: NewWorkspaceState("docapp"), Dispatcher: d}
+	res, err := makeFanOutHandler(padItemTool, env)(context.Background(), callToolRequest(in))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if res != nil && res.IsError {
+		if d.gotPath != nil {
+			t.Fatalf("refused but dispatched: %v", d.gotArgs)
+		}
+		return "", textOf(res)
+	}
+	for i := 0; i+1 < len(d.gotArgs); i++ {
+		if d.gotArgs[i] == "--edits" {
+			return canonicalJSON(t, d.gotArgs[i+1]), ""
+		}
+	}
+	t.Fatalf("no --edits forwarded: %v", d.gotArgs)
+	return "", ""
+}
+
+func TestBUG4082_LineHintSameOnBothTransports(t *testing.T) {
+	for _, tc := range bug4082Cases {
+		t.Run(tc.name, func(t *testing.T) {
+			rFwd, rRef := remoteLineHints(t, tc.edits)
+			sFwd, sRef := stdioLineHints(t, tc.edits)
+			if tc.refuse != "" {
+				// The JSON envelope escapes quotes, so compare stdio against the
+				// escaped form of remote's error text.
+				escaped, _ := json.Marshal(rRef)
+				if !strings.Contains(rRef, strings.ReplaceAll(tc.refuse, `\"`, `"`)) {
+					t.Errorf("remote: want refusal containing %q, got %q (forwarded %s)", tc.refuse, rRef, rFwd)
+				}
+				if !strings.Contains(sRef, string(ErrValidationFailed)) ||
+					!strings.Contains(sRef, "item patch: "+strings.Trim(string(escaped), `"`)) {
+					t.Errorf("stdio: want validation_failed with remote's message %q, got %q (forwarded %s)", rRef, sRef, sFwd)
+				}
+				// Checked on its own too, so a helper change that alters both
+				// messages alike still turns stdio red (C1). tc.refuse is written
+				// in the envelope's escaped form.
+				if !strings.Contains(sRef, tc.refuse) {
+					t.Errorf("stdio: want refusal containing %q, got %q", tc.refuse, sRef)
+				}
+				return
+			}
+			want := canonicalJSON(t, tc.want)
+			if rRef != "" || rFwd != want {
+				t.Errorf("remote: want edits %s, got %s (refusal %q)", want, rFwd, rRef)
+			}
+			if sRef != "" || sFwd != want {
+				t.Errorf("stdio: want edits %s, got %s (refusal %q)", want, sFwd, sRef)
+			}
+		})
+	}
+}
+
+// C2: neither transport modifies the caller's map, its edits slice or an
+// edit in it, on a coerced hint or on a refusal.
+func TestBUG4082_CallerMapUnmodified(t *testing.T) {
+	for _, edits := range []string{
+		`[{"old":"a","new":"b","line_hint":"5"},"x"]`,
+		`[{"old":"a","new":"b","line_hint":2},{"old":"c","new":"d","line_hint":"010"}]`,
+	} {
+		t.Run("remote "+edits, func(t *testing.T) {
+			in, before := bug4082Input(t, edits), bug4082Input(t, edits)
+			_, _, _, _ = mapItemPatch(in)
+			if !reflect.DeepEqual(in, before) {
+				t.Fatalf("caller's map modified: %v, was %v", in, before)
+			}
+		})
+		t.Run("stdio "+edits, func(t *testing.T) {
+			in, before := bug4082Input(t, edits), bug4082Input(t, edits)
+			in["action"], before["action"] = "patch", "patch"
+			env := ActionEnv{Doc: liveCmdhelpDoc(t), Workspace: NewWorkspaceState("docapp"), Dispatcher: &fakeDispatcher{}}
+			req := callToolRequest(in)
+			if _, err := makeFanOutHandler(padItemTool, env)(context.Background(), req); err != nil {
+				t.Fatal(err)
+			}
+			if !reflect.DeepEqual(in, before) {
+				t.Fatalf("caller's map modified: %v, was %v", in, before)
+			}
+		})
+		// The fan-out handler hands actions a private copy, so the row above
+		// cannot see actionItemPatch touch its own input. Call it directly.
+		t.Run("stdio action "+edits, func(t *testing.T) {
+			in, before := bug4082Input(t, edits), bug4082Input(t, edits)
+			env := ActionEnv{Doc: liveCmdhelpDoc(t), Workspace: NewWorkspaceState("docapp"), Dispatcher: &fakeDispatcher{}}
+			if _, err := actionItemPatch(context.Background(), in, env); err != nil {
+				t.Fatal(err)
+			}
+			if !reflect.DeepEqual(in, before) {
+				t.Fatalf("actionItemPatch modified its input: %v, was %v", in, before)
+			}
+		})
+	}
+}

internal/mcp/catalog_item.go

@@ -62,7 +62,8 @@ var padItemTool = ToolDef{
 		"get":    actionItemGet,
 
 		// PLAN-3578: content-anchored body edits, answered with the diff.
-		"patch": passThrough([]string{"item", "patch"}),
+		// CUSTOM (BUG-4082) so each edit's line_hint gets remote's rule.
+		"patch": actionItemPatch,
 		// list is CUSTOM (not passThrough) so a bare agent list stays
 		// bounded: it injects a default limit and clamps an oversized one
 		// before dispatch, mirroring the backlinks default/max. See
@@ -770,6 +771,32 @@ const (
 	mcpItemListMaxLimit     = 300
 )
 
+// actionItemPatch handles pad_item.action=patch on stdio. It gives each edit's
+// line_hint remote's rule (editsWithLineHints, shared with mapItemPatch) and
+// then forwards to `item patch` as passThrough did (BUG-4082). edits used to
+// reach the CLI unread: its JSON decode into an int refused a string hint that
+// remote coerces, and took a JSON number of 2^53 or more, which mcp-go's
+// float64 decode of the request may already have rounded, as a line number. A
+// refusal is validation_failed with remote's message. The caller's map is not
+// modified.
+func actionItemPatch(ctx context.Context, input map[string]any, env ActionEnv) (*CallToolResult, error) {
+	edits, isList := input["edits"].([]any)
+	if !isList {
+		return env.Dispatch(ctx, []string{"item", "patch"}, input)
+	}
+	out, err := editsWithLineHints(edits)
+	if err != nil {
+		return validationFailedResult("item patch", err.Error(),
+			"Pass each line_hint as a whole JSON number below 2^53, or a string of decimal digits with no leading zero."), nil
+	}
+	in := make(map[string]any, len(input))
+	for k, v := range input {
+		in[k] = v
+	}
+	in["edits"] = out
+	return env.Dispatch(ctx, []string{"item", "patch"}, in)
+}
+
 // actionItemList handles pad_item.action=list. It forwards to `item list`
 // but first applies a default limit (when the agent didn't ask for one) and
 // clamps an oversized one, so a bare agent list can't dump the whole

internal/mcp/dispatch_http_routes.go

@@ -886,31 +886,52 @@ func mapItemPatch(input map[string]any) (string, string, []byte, error) {
 		in["expected_seq"] = seq
 	}
 	if edits, isList := input["edits"].([]any); isList {
-		out := make([]any, len(edits))
-		for i, e := range edits {
-			out[i] = e
-			edit, isObj := e.(map[string]any)
-			if !isObj {
-				continue
-			}
-			hint, present, err := intParam(edit, "line_hint")
-			if err != nil {
-				return "", "", nil, fmt.Errorf("%w (in edits[%d])", err, i)
-			}
-			if present {
-				copied := make(map[string]any, len(edit))
-				for k, v := range edit {
-					copied[k] = v
-				}
-				copied["line_hint"] = hint
-				out[i] = copied
-			}
+		out, err := editsWithLineHints(edits)
+		if err != nil {
+			return "", "", nil, err
 		}
 		in["edits"] = out
 	}
 	return itemPatchRoute(in)
 }
 
+// editsWithLineHints returns a copy of a patch's edits with each object edit's
+// line_hint read through intParam (numericInput's rule) and replaced by its
+// int64. A refusal names the edit, e.g. `line_hint must be a whole number, got
+// the string "010" (in edits[0])`. An edit that is not an object is left as it
+// is, for the server or the CLI to refuse, since its shape is theirs to
+// define; a null line_hint is absent and left as it is. Neither edits nor any
+// edit in it is modified: a changed edit is a copy.
+//
+// Both transports call it (BUG-4082): remote's mapItemPatch, where the loop
+// was inline (BUG-4065), and stdio's patch action, which used to forward edits
+// to the CLI unread, so a string hint was refused there where remote coerced
+// it, and a JSON number of 2^53 or more, already rounded by the decoder,
+// reached bodypatch as a line number.
+func editsWithLineHints(edits []any) ([]any, error) {
+	out := make([]any, len(edits))
+	for i, e := range edits {
+		out[i] = e
+		edit, isObj := e.(map[string]any)
+		if !isObj {
+			continue
+		}
+		hint, present, err := intParam(edit, "line_hint")
+		if err != nil {
+			return nil, fmt.Errorf("%w (in edits[%d])", err, i)
+		}
+		if present {
+			copied := make(map[string]any, len(edit))
+			for k, v := range edit {
+				copied[k] = v
+			}
+			copied["line_hint"] = hint
+			out[i] = copied
+		}
+	}
+	return out, nil
+}
+
 // mapItemClaim dispatches `pad item claim <ref> [--holder] [--ttl]`.
 //
 // POST /api/v1/workspaces/{ws}/items/{ref}/claim with an optional body

internal/mcp/instructions.md

@@ -6,7 +6,7 @@ Pad is a project tracker for developers and AI agents — issues (TASK, BUG), pl
 
 If the user is asking general code questions with no project-management thread, you don't need this server.
 
-## Tool surface (v0.105)
+## Tool surface (v0.106)
 
 Eleven resource × action tools, plus `pad_set_workspace`. Twelve tools total.
 

internal/mcp/version.go

@@ -2284,7 +2284,19 @@ const CmdhelpVersion = "0.1"
 // bool, an array or an object; everywhere else the CLI's own parse refuses
 // all of them. The nested pad_item.patch edits[].line_hint is BUG-4082.
 // Behaviour only; the schemas are unchanged, as for v0.103 and v0.104.
-const ToolSurfaceVersion = "0.105"
+//
+// v0.106 — BUG-4082: stdio's pad_item.patch gives each edit's line_hint
+// remote's rule (v0.104), through the one helper both transports now call.
+// stdio used to forward edits to the CLI unread, so its JSON decode refused a
+// string hint that remote coerces ("5" is now 5 on stdio too), and took 2^53
+// or more, which mcp-go's float64 decode of the request may already have
+// rounded, as a line number. Now
+// a hint that is not a whole number below 2^53 (a fraction, "010", a bool, an
+// object) is refused on stdio with remote's validation_failed, naming the
+// edit ("(in edits[1])"); null is absent, and an edit that is not an object
+// keeps its path. Remote is unchanged. Behaviour only; the schemas are
+// unchanged.
+const ToolSurfaceVersion = "0.106"
 
 // MetaVersionURI is the canonical URI of the queryable version document.
 // Lives outside the pad://workspace/{ws}/... namespace because it's a