Repository navigation
Description
Problem
fieldInMessage in pkgs/mcpx/internal/diagnose — the function that decides which declared argument a server's rejection message "wants" — iterates the shape's property map and keeps the longest matching name, but breaks equal-length ties by Go map iteration order. When a message names two same-length fields in the same class (both bare tokens, or both quoted), the blamed field changes from run to run, and with it the diagnostic's Field, its "it wants <type>" text, and its Changed attribution.
Evidence (verified this run on main facf6481)
pkgs/mcpx/internal/diagnose/diagnose.go:582-607—fieldInMessagerangesfor name := range shape.Props(a map) and replaces the incumbent only whenlen(name) > len(best)within the same quoted class (lines 597-603: quoted-vs-bare mismatch handled first, thenlen(name) <= len(best)skips). Nothing orders equal-length candidates, and Go map range order is deliberately random per iteration.- The doc comment (diagnose.go:571-580) specifies the ranking — quoted occurrence outranks bare token, "within a class the longest name wins" — and is silent on ties, so the tie outcome is unspecified and unstable.
- The result is load-bearing at the only call site,
diagnose.go:523-533: the returned field becomesDiagnostic.Field, is interpolated into the message (the server rejected %s (it wants %s)), and drivescat.change(tool.Path(), field).
First-hand probe on the laptop (the four functions copied verbatim from current main into a scratch module; Shape stubbed with the same map type), same input, 4000 calls in one process:
msg "user and name are incompatible; host and port conflict", props {user,name,host,port} (all 4 chars, all bare tokens):
run 1: map[host:514 name:498 port:507 user:2481]
run 2: map[host:498 name:514 port:473 user:2515]
msg "'user' and 'name' are both wrong", props {user,name} (both quoted):
run 1: map[name:484 user:3516]
run 2: map[name:472 user:3528]
Every tied candidate wins some iterations; nothing but map order varies. (The skew toward the first-inserted key is Go's small-map iteration bias — the point is that all outcomes occur.)
Why it matters
mcpx diagnostics are the user-facing explanation of a failed call, and the fleet's own standard elsewhere in this codebase is deterministic tie-breaking: nearmiss.Nearest (internal/nearmiss/nearmiss.go:105-108) explicitly breaks ties lexicographically "so the answer never depends on the order candidates were enumerated in". fieldInMessage missed that treatment when #1646/#3041 rebuilt its matching: the same failed call can blame user in one run and name in the next, pointing the user (and any test or snapshot built on the diagnostic) at different arguments and different schema types for identical input. It also makes any future regression test over a tie case flaky by construction.
Acceptance criteria
-
fieldInMessageis deterministic: for a fixed message and shape, repeated calls always return the same field, including when two or more same-length fields match in the same class (bare or quoted). The tie rule is stated in the doc comment (e.g. lexicographically smallest name, matchingnearmiss.Nearest, or first occurrence in the message — either is acceptable if documented and tested). - The existing ranking is preserved: a quoted occurrence still outranks a bare token, and a longer name still beats a shorter one within a class; the mcpx(diagnose): CallError blames a field the server never named — fieldInMessage substring-matches prop names inside unrelated words ('valid' in 'invalid' beats the named 'id') #1646 token-boundary and empty-name behaviours are unchanged.
- Regression tests in
internal/diagnosecover a bare-token tie and a quoted tie, asserting one stable winner across repeated calls (e.g. loop the call and require a single outcome). -
go test ./internal/diagnose -run 'TestFieldInMessage|TestDiagnose' -count=1(or the package's equivalent targeted tests) passes on laptop or CI.
Dedup
Searched open + closed issues for fieldInMessage tie / nondeterministic / map: the only hit is #1646 (closed — the substring/token-boundary rebuild this function came from, including the #3041 quote-preference and empty-name fixes). Neither it nor any other issue covers equal-length tie determinism; the defect is present on current main.
Activity
dezren39 commentedon Oct 31, 2026
🤖 Agent note · state: filed
4 remaining items
dezren39 commentedon Nov 1, 2026
🤖 Agent note · done (triage)
dezren39 commentedon Nov 1, 2026
🤖 Agent note · picked up
fieldInMessagematches deterministic with a lexicographic tie-break, preserving existing ranking rules.fix(mcpx): stabilize diagnose field ties (#3103)
dezren39 commentedon Nov 1, 2026
🤖 Agent note · done
go test ./internal/diagnosepassed; bare and quoted tie tests each repeat the same call 100 times; existing quote preference and longest-wins controls remain.queue/5634-3103-field-tieMerge pull request #3713 from developing-today/queue/5634-3103-field-tie