fix(mcp): integral numbers keep decimal notation on both transports (BUG-4056) (#2300)

* fix(mcp): preserve decimal numeric arguments (BUG-4056)

Share numeric formatting across CLI dispatch and HTTP queries, and validate remote chat sequences directly with int64 bounds. Preserve the existing tool surface and wire goldens.

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

* fix(mcp): numeric array elements, playbook tokens and buildQuery's default share formatNumber (BUG-4056, r1)

Lead ruling on r1 (tier A from here): toStringSlice spelled each JSON number
element of an array with fmt.Sprint, so a repeatable flag or variadic
positional given 1000000 got '1e+06'; playbook raw_args tokens used %v, so a
string- or ref-typed argument would bind '1e+06'; buildQuery's default
branch kept fmt.Sprint for float32. All three now use formatNumber.

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

cmd/pad/bug4056_mcp_numbers_test.go

@@ -0,0 +1,36 @@
+package main
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/PerpetualSoftware/pad/internal/cmdhelp"
+	mcpserver "github.com/PerpetualSoftware/pad/internal/mcp"
+)
+
+func TestBUG4056BuildCLIArgs(t *testing.T) {
+	root := newRootCmd()
+	doc := cmdhelp.Build(root, root, cmdhelp.Options{Binary: "pad", MaxDepth: -1})
+	for _, tc := range []struct {
+		name, command string
+		input         map[string]any
+		want          []string
+	}{
+		{"flag", "chat read", map[string]any{"target": "C12", "to": float64(1000000)}, []string{"--to", "1000000", "--format", "json", "--", "C12"}},
+		{"positional", "chat delete", map[string]any{"target": "C12", "seq": float64(1000000)}, []string{"--format", "json", "--", "C12", "1000000"}},
+		// BUG-4056 r1: a repeatable positional given a JSON array of numbers
+		// goes through toStringSlice, element by element.
+		{"repeatable positional", "item bulk-update", map[string]any{"ref": []any{float64(1000000), float64(2.5)}}, []string{"--format", "json", "--", "1000000", "2.5"}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			cmd, ok := doc.Commands[tc.command]
+			if !ok {
+				t.Fatalf("catalog command %q missing from cmdhelp", tc.command)
+			}
+			got, err := mcpserver.BuildCLIArgs(cmd, tc.input, "", nil)
+			if err != nil || !reflect.DeepEqual(got, tc.want) {
+				t.Fatalf("BuildCLIArgs = %q, %v; want %q", got, err, tc.want)
+			}
+		})
+	}
+}

docs/mcp.md

@@ -30,6 +30,8 @@ Surface:
 
 Post-0.30 without a bump (BUG-2995): a successful content write through the designated applier used to answer with the item's PREVIOUS content — on that path the markdown goes to a live tab's Y.Doc and the row write runs with `Content` nil, so the response described the item as it stood before the request. It now carries the content as SENT plus an additive `omitempty` `warnings.content_outcome: "applied_pending_flush"`. No bump, on the BUG-2304 rather than the v0.9 grounds: v0.9 bumped because list rows LOST fields consumers read, whereas here nothing is removed or retyped — what changed is the VALUE of one field on one path, which a caller does observe. The claim is that no DOCUMENTED OR SUPPORTED reliance breaks, not that none can exist: a consumer could have detected the applier path by noticing the response echoed something other than what it sent, and that stops working — but it was never documented, a genuinely lost write produced the same mismatch, and `warnings.content_outcome` answers that question properly in the same change. Reads are unchanged: during the window a `get` (or `list` with `full: true`) still answers from the row, so the warning means re-read later, not re-send.
 
+Post-0.102 without a bump (BUG-4056): calls with integral numbers >= 1e6 that were wrongly refused now work. Numeric CLI flags and positionals retain decimal notation, and remote `pad_chat` validates sequences directly from the number. No schema, name, or shape change.
+
 Both are also returned by `pad://_meta/version` and `pad_meta.action: version`.
 
 **The wire contract is pinned as golden bytes** (TASK-2306): `cmd/pad/mcp_wire_golden_test.go` compares the `initialize` and `tools/list` results, on BOTH transports, against `cmd/pad/testdata/mcp_wire/*.json`. It uses the production bindings: the real `pad mcp serve` subcommand, and `registerRemoteMCP` behind `NewRemoteTransport`. Any catalog, annotation, schema or `instructions.md` edit moves those bytes. Regenerate with `PAD_UPDATE_MCP_GOLDEN=1 go test ./cmd/pad/ -run TestMCPWireGolden`, and review the testdata diff as the contract diff it is: that diff is where you decide whether the change owes a `ToolSurfaceVersion` bump. A refactor or SDK bump that is meant to be behavior-neutral must leave the files untouched.

internal/mcp/bug4056_numbers_test.go

@@ -0,0 +1,93 @@
+package mcp
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"math"
+	"net/http"
+	"net/url"
+	"strings"
+	"testing"
+
+	"github.com/PerpetualSoftware/pad/internal/models"
+)
+
+func TestBUG4056BuildQueryNumbersUnchanged(t *testing.T) {
+	for _, tc := range []struct {
+		name string
+		in   any
+		want string
+	}{
+		{"integral", float64(1000000), "1000000"},
+		{"large integral", float64(1e19), "10000000000000000000"},
+		{"fractional", 2.5, "2.5"},
+		{"negative zero", math.Copysign(0, -1), "0"},
+		{"json number", json.Number("1e6"), "1e6"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			query := buildQuery(map[string]any{"n": tc.in}, map[string]string{"n": "n"})
+			values, err := url.ParseQuery(query)
+			if err != nil || values.Get("n") != tc.want {
+				t.Fatalf("buildQuery = %q, %v; want n=%q", query, err, tc.want)
+			}
+		})
+	}
+}
+
+func TestBUG4056ChatRemoteNumbers(t *testing.T) {
+	for _, tc := range []struct {
+		name, verb, field string
+		in                any
+		invalid           bool
+	}{
+		{"million", "read", "to", float64(1000000), false},
+		{"string", "read", "to", "1000000", false},
+		{"int", "read", "to", int(1000000), false},
+		{"int64", "read", "to", int64(1000000), false},
+		{"out of range", "delete", "seq", float64(1e19), true},
+		{"int64 upper bound", "delete", "seq", float64(math.MaxInt64), true},
+		{"fractional", "delete", "seq", 2.5, true},
+		{"nan", "delete", "seq", math.NaN(), true},
+		{"infinity", "delete", "seq", math.Inf(1), true},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			calls := 0
+			d := &HTTPHandlerDispatcher{
+				UserResolver: func(context.Context) *models.User { return &models.User{ID: "u"} },
+				Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					calls++
+					if tc.invalid {
+						t.Error("invalid sequence reached handler")
+					}
+					if r.Method != http.MethodPost || r.URL.Path != "/api/v1/me/conversations/12/read" {
+						t.Errorf("unexpected request: %s %s", r.Method, r.URL)
+					}
+					var body map[string]json.RawMessage
+					if err := json.NewDecoder(r.Body).Decode(&body); err != nil || string(body["seq"]) != "1000000" {
+						t.Errorf("request body = %s, %v; want seq=1000000", body, err)
+					}
+					fmt.Fprint(w, `{"read_seq":1000000}`)
+				}),
+			}
+			// @name forces target resolution to make a request if validation
+			// ever moves after it; invalid input must reach no handler at all.
+			target := "C12"
+			if tc.invalid {
+				target = "@rook"
+			}
+			input := map[string]any{"target": target, tc.field: tc.in}
+			res, err := d.Dispatch(WithDispatchInput(context.Background(), input), []string{"chat", tc.verb}, nil)
+			if err != nil || res == nil {
+				t.Fatalf("Dispatch = %v, %v", res, err)
+			}
+			if tc.invalid {
+				if !res.IsError || !strings.Contains(textOf(res), "validation_failed") || !strings.Contains(textOf(res), "seq must be an integer of at least 1") || calls != 0 {
+					t.Fatalf("want validation error without requests; calls=%d result=%s", calls, textOf(res))
+				}
+			} else if res.IsError || calls != 1 {
+				t.Fatalf("want successful read request; calls=%d result=%s", calls, textOf(res))
+			}
+		})
+	}
+}

internal/mcp/catalog_playbook.go

@@ -169,6 +169,12 @@ func actionPlaybookRun(ctx context.Context, input map[string]any, env ActionEnv)
 				// Skip — caller signaled "leave unbound"; let the
 				// server's bindPlaybookArgs surface it as unbound.
 			default:
+				// BUG-4056 r1: a number-typed argument parses '1e+06' fine,
+				// but a string- or ref-typed one would bind that text.
+				if number, ok := formatNumber(tv); ok {
+					tokens = append(tokens, k+"="+number)
+					continue
+				}
 				tokens = append(tokens, fmt.Sprintf("%s=%v", k, tv))
 			}
 		}

internal/mcp/dispatch.go

@@ -502,6 +502,8 @@ func BuildCLIArgs(
 				return nil, fmt.Errorf("argument %q: %w", arg.Name, err)
 			}
 			positionals = append(positionals, items...)
+		} else if number, ok := formatNumber(v); ok {
+			positionals = append(positionals, number)
 		} else {
 			positionals = append(positionals, fmt.Sprint(v))
 		}
@@ -653,6 +655,11 @@ func toStringSlice(v any) ([]string, error) {
 	case []any:
 		out := make([]string, 0, len(t))
 		for _, e := range t {
+			// BUG-4056 r1: a JSON number element is a float64 too.
+			if number, ok := formatNumber(e); ok {
+				out = append(out, number)
+				continue
+			}
 			out = append(out, fmt.Sprint(e))
 		}
 		return out, nil
@@ -663,8 +670,8 @@ func toStringSlice(v any) ([]string, error) {
 }
 
 // encodeFlagValue stringifies an MCP input value for inclusion as a CLI
-// flag argument. Strings and primitives pass through fmt.Sprint as before;
-// structured values (maps, slices) are JSON-encoded so that flags accepting
+// flag argument. Numbers use decimal notation; strings and booleans pass through.
+// Structured values (maps, slices) are JSON-encoded so that flags accepting
 // JSON literals — like `pad collection create --schema '<json>'` — receive
 // a valid JSON string rather than Go's map-print syntax.
 //
@@ -672,12 +679,15 @@ func toStringSlice(v any) ([]string, error) {
 // involved; the JSON string travels intact to the subprocess and is parsed
 // by the CLI flag's own decoder.
 func encodeFlagValue(v any) (string, error) {
+	if number, ok := formatNumber(v); ok {
+		return number, nil
+	}
 	switch t := v.(type) {
 	case nil:
 		return "", nil
 	case string:
 		return t, nil
-	case bool, int, int32, int64, uint, uint32, uint64, float32, float64, json.Number:
+	case bool:
 		return fmt.Sprint(v), nil
 	default:
 		// map[string]any, []any, or any other structured type: JSON-encode.

internal/mcp/dispatch_http_chat.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"math"
 	"net/http"
 	"net/url"
 	"strconv"
@@ -100,7 +101,17 @@ func chatSequence(input map[string]any, name string, min int64) (int64, error) {
 	if !ok {
 		return 0, fmt.Errorf("%s is required", name)
 	}
-	n, err := strconv.ParseInt(fmt.Sprint(v), 10, 64)
+	var n int64
+	var err error
+	if x, ok := v.(float64); ok {
+		// The upper bound is exclusive: float64(MaxInt64) is 2^63.
+		if math.Trunc(x) != x || x < math.MinInt64 || x >= 0x1p63 {
+			return 0, fmt.Errorf("%s must be an integer of at least %d", name, min)
+		}
+		n = int64(x)
+	} else {
+		n, err = strconv.ParseInt(fmt.Sprint(v), 10, 64)
+	}
 	if err != nil || n < min {
 		return 0, fmt.Errorf("%s must be an integer of at least %d", name, min)
 	}

internal/mcp/dispatch_http_routes.go

@@ -153,19 +153,17 @@ func buildQuery(input map[string]any, mapping map[string]string) string {
 				q.Set(dst, "true")
 			}
 		case float64:
-			// Cheap int detection — JSON parser gives every number as
-			// float64, but CLI flags in cmdhelp can be "int" type so
-			// most callers pass whole numbers. Emit without the
-			// trailing ".0" so the wire format matches the CLI.
-			if x == float64(int64(x)) {
-				q.Set(dst, strconv.FormatInt(int64(x), 10))
-			} else {
-				q.Set(dst, strconv.FormatFloat(x, 'f', -1, 64))
-			}
+			number, _ := formatNumber(x)
+			q.Set(dst, number)
 		case json.Number:
 			q.Set(dst, x.String())
 		default:
-			q.Set(dst, fmt.Sprint(v))
+			// BUG-4056 r1: float32 and the integer types share formatNumber.
+			if number, ok := formatNumber(v); ok {
+				q.Set(dst, number)
+			} else {
+				q.Set(dst, fmt.Sprint(v))
+			}
 		}
 	}
 	if len(q) == 0 {

internal/mcp/number.go

@@ -0,0 +1,42 @@
+package mcp
+
+import (
+	"encoding/json"
+	"math"
+	"strconv"
+)
+
+// formatNumber formats native numbers without exponents and preserves
+// json.Number literals. NaN and infinities keep strconv's spellings so callers
+// can apply their own validation, as they did with fmt.Sprint.
+func formatNumber(v any) (string, bool) {
+	var x float64
+	switch n := v.(type) {
+	case float64:
+		x = n
+	case float32:
+		x = float64(n)
+	case int:
+		return strconv.Itoa(n), true
+	case int32:
+		return strconv.FormatInt(int64(n), 10), true
+	case int64:
+		return strconv.FormatInt(n, 10), true
+	case uint:
+		return strconv.FormatUint(uint64(n), 10), true
+	case uint32:
+		return strconv.FormatUint(uint64(n), 10), true
+	case uint64:
+		return strconv.FormatUint(n, 10), true
+	case json.Number:
+		return n.String(), true
+	default:
+		return "", false
+	}
+	// MaxInt64 rounds up to 2^63 as a float64, so the upper bound is
+	// exclusive. Converting only within range also normalizes -0 to 0.
+	if math.Trunc(x) == x && x >= math.MinInt64 && x < 0x1p63 {
+		return strconv.FormatInt(int64(x), 10), true
+	}
+	return strconv.FormatFloat(x, 'f', -1, 64), true
+}

internal/mcp/number_test.go

@@ -0,0 +1,80 @@
+package mcp
+
+import (
+	"context"
+	"encoding/json"
+	"math"
+	"strings"
+	"testing"
+)
+
+func TestFormatNumber(t *testing.T) {
+	for _, tc := range []struct {
+		name string
+		in   any
+		want string
+		ok   bool
+	}{
+		{"million", float64(1000000), "1000000", true},
+		{"one and a half million", float64(1500000), "1500000", true},
+		{"quadrillion", float64(1e15), "1000000000000000", true},
+		{"outside int64", float64(1e19), "10000000000000000000", true},
+		{"fractional", 2.5, "2.5", true},
+		{"negative", float64(-3), "-3", true},
+		{"negative zero", math.Copysign(0, -1), "0", true},
+		{"last float below int64 upper bound", math.Nextafter(0x1p63, 0), "9223372036854774784", true},
+		{"int64 upper bound", float64(0x1p63), "9223372036854776000", true},
+		{"int64 minimum", float64(math.MinInt64), "-9223372036854775808", true},
+		{"below int64 minimum", math.Nextafter(-0x1p63, math.Inf(-1)), "-9223372036854778000", true},
+		{"float32 integral", float32(1000000), "1000000", true},
+		{"float32 fractional", float32(0.1), "0.10000000149011612", true},
+		{"float32 outside int64", float32(0x1p64), "18446744073709552000", true},
+		{"float32 negative zero", float32(math.Copysign(0, -1)), "0", true},
+		{"int", int(7), "7", true},
+		{"int32", int32(7), "7", true},
+		{"int64", int64(7), "7", true},
+		{"uint", uint(7), "7", true},
+		{"uint32", uint32(7), "7", true},
+		{"uint64", uint64(math.MaxUint64), "18446744073709551615", true},
+		{"json number", json.Number("7"), "7", true},
+		{"json exponent preserved", json.Number("1e6"), "1e6", true},
+		{"nan", math.NaN(), "NaN", true},
+		{"positive infinity", math.Inf(1), "+Inf", true},
+		{"negative infinity", math.Inf(-1), "-Inf", true},
+		{"string", "7", "", false},
+		{"bool", true, "", false},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			got, ok := formatNumber(tc.in)
+			if got != tc.want || ok != tc.ok {
+				t.Fatalf("formatNumber(%v) = %q, %v; want %q, %v", tc.in, got, ok, tc.want, tc.ok)
+			}
+		})
+	}
+}
+
+// BUG-4056 r1: array elements and playbook argument tokens share the helper.
+func TestBUG4056SliceAndPlaybookNumbers(t *testing.T) {
+	got, err := toStringSlice([]any{float64(1000000), float64(2.5), "x", true})
+	if err != nil || strings.Join(got, ",") != "1000000,2.5,x,true" {
+		t.Fatalf("toStringSlice = %q, %v", got, err)
+	}
+
+	disp := &fakeDispatcher{}
+	env := ActionEnv{Doc: liveCmdhelpDoc(t), Workspace: NewWorkspaceState("docapp"), Dispatcher: disp}
+	if _, err := padPlaybookTool.Actions["run"](context.Background(), map[string]any{
+		"ref":  "release",
+		"args": map[string]any{"n": float64(1000000), "version": float64(1500000)},
+	}, env); err != nil {
+		t.Fatalf("dispatch error: %v", err)
+	}
+	joined := strings.Join(disp.gotArgs, " ")
+	for _, want := range []string{"n=1000000", "version=1500000"} {
+		if !strings.Contains(joined, want) {
+			t.Fatalf("playbook tokens %q lack %q", joined, want)
+		}
+	}
+	if strings.Contains(joined, "e+06") {
+		t.Fatalf("playbook tokens carry exponent text: %q", joined)
+	}
+}