fix(mcp): canonicalization is a per-KEY property, not a per-request one (BUG-2850)

Codex round 17, one P1, and it is the third consecutive round where my
own fix generalized a property verified on one subset to the whole.

Round 16 gated the padded-entry refusal on "no `fields` object",
reasoning that a `fields` object makes reshapeItemFields re-emit the
entry canonically. That holds for keys IN that object. With `fields:{}`,
or a `fields` carrying some OTHER key, nothing canonicalizes
`field:["status = done"]` and it reaches the doors padded exactly as it
does with no `fields` at all — HTTP writes `status`, the CLI writes a
junk `"status "` beside it.

The predicate is now the actual question: will anything canonicalize
THIS key.

The pattern is worth naming because it is now a habit rather than an
accident:

  round 15 — a premise true of schema-declared params, applied to the
             compat IDs, which are undeclared precisely so it cannot hold
  round 16 — the check placed below the exemption, so it covered one key
             class (caught by my own test's control leg)
  round 17 — a per-key property read as per-request

Each time the fix was correct for the case in front of me and wrong for
its siblings, which is the same shape as the defects this unit started
with. The canonical restructure removed it from the CODE; it evidently
did not remove it from how I reason about the code.

Mutation matrix, both directions:

  revert to the per-request gate     -> only the two uncovered-key legs fail
  refuse even when canonicalized     -> only the canonicalized control fails

The third leg of the new test is the control that makes the distinction
real: with the key present in `fields` the entry IS canonicalized, so
the call must still succeed. A fix that refused whenever anything was
padded passes the first two legs and fails this one.

Gates: gofmt clean · go vet clean · go test ./... green (29 packages) ·
the contract-drift gate ran green
(go test ./internal/mcp/ -run 'CoversEveryCatalogAction|VersionMatchesToolSurface')

internal/mcp/catalog_item_fields.go

@@ -301,7 +301,30 @@ func detectFieldConflicts(prefix string, input map[string]any) *mcp.CallToolResu
 		// and it changes what every CLI caller receives. Here the caller has
 		// supplied one key twice and one of the forms is malformed, which is
 		// a narrower and locally-answerable question.
-		if !fieldsPresent {
+		// WHETHER THIS KEY GETS CANONICALIZED IS A PER-KEY QUESTION, not a
+		// per-request one (codex round 17).
+		//
+		// Round 16 gated this on `!fieldsPresent`, reasoning that with a
+		// `fields` object present reshapeItemFields re-emits the entry
+		// canonically. True — for keys that are IN that object. With
+		// `fields:{}`, or a `fields` carrying some OTHER key, nothing
+		// canonicalizes `field:["status = done"]` and it reaches the doors
+		// padded exactly as it does with no `fields` at all.
+		//
+		// Third round running that I generalized a property verified on one
+		// subset to the whole: round 15 (a premise true of declared params,
+		// applied to the compat IDs), round 16's first draft (a check placed
+		// below the exemption so it covered one key class), and now a
+		// per-key property read as per-request. The predicate is now the
+		// actual question — will anything canonicalize THIS key.
+		canonicalized := false
+		for _, c := range contribs {
+			if _, inObj := obj[c.key]; inObj {
+				canonicalized = true
+				break
+			}
+		}
+		if !canonicalized {
 			for i := 1; i < len(contribs); i++ {
 				a, b := contribs[0], contribs[i]
 				if a.nonCanonical || b.nonCanonical {

internal/mcp/catalog_item_fields_test.go

@@ -1805,3 +1805,56 @@ func TestPadItemUpdate_CanonicalCompatDuplicateStillCollapses(t *testing.T) {
 		t.Fatal("expected the update to dispatch")
 	}
 }
+
+// --- codex round 17 ---
+
+// TestPadItemUpdate_PaddedEntryRefusedWhenFieldsDoesNotCoverTheKey: whether an
+// entry gets canonicalized is a PER-KEY question (BUG-2850, codex round 17).
+//
+// Round 16 gated the padded-entry refusal on "no `fields` object", reasoning
+// that a `fields` object causes reshapeItemFields to re-emit the entry
+// canonically. That holds for keys IN the object. With `fields:{}` or a
+// `fields` carrying some other key, nothing canonicalizes
+// `field:["status = done"]` and it reaches the doors padded exactly as it
+// does with no `fields` at all — HTTP writes `status`, the CLI writes a junk
+// `"status "` beside it.
+//
+// The last leg is the control that makes the distinction real: with the key
+// present in `fields`, the entry IS canonicalized, so the call must still
+// succeed. A fix that simply refused whenever anything was padded would pass
+// the first two legs and fail this one.
+func TestPadItemUpdate_PaddedEntryRefusedWhenFieldsDoesNotCoverTheKey(t *testing.T) {
+	t.Run("fields empty", func(t *testing.T) {
+		disp, msg, isErr := dispatchPadItem(t, map[string]any{
+			"action": "update", "ref": "TASK-5",
+			"status": "done", "field": []any{"status = done"},
+			"fields": map[string]any{},
+		})
+		if !isErr {
+			t.Fatalf("nothing canonicalizes this entry; expected refusal, got: %s (args %v)", msg, disp.gotArgs)
+		}
+	})
+	t.Run("fields carries another key", func(t *testing.T) {
+		disp, msg, isErr := dispatchPadItem(t, map[string]any{
+			"action": "update", "ref": "TASK-5",
+			"status": "done", "field": []any{"status = done"},
+			"fields": map[string]any{"effort": "l"},
+		})
+		if !isErr {
+			t.Fatalf("a fields object covering a DIFFERENT key does not canonicalize this one; got: %s (args %v)", msg, disp.gotArgs)
+		}
+	})
+	t.Run("fields carries the key — canonicalized, so accepted", func(t *testing.T) {
+		disp, msg, isErr := dispatchPadItem(t, map[string]any{
+			"action": "update", "ref": "TASK-5",
+			"status": "done", "field": []any{"status = done"},
+			"fields": map[string]any{"status": "done"},
+		})
+		if isErr {
+			t.Fatalf("this key IS canonicalized, so the call must succeed: %s", msg)
+		}
+		if argsContainPair(disp.gotArgs, "--field", "status = done") {
+			t.Errorf("the padded entry must not survive canonicalization: %v", disp.gotArgs)
+		}
+	})
+}