Skip to content

fix(mcpx): Clamp validates request against the dial's own enum first (#1641) #2883

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dezren39 merged 1 commit into from
Nov 2, 2026
Merged

fix(mcpx): Clamp validates request against the dial's own enum first (#1641) #2883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkgs/mcpx/internal/settings/clamp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ func (s *Set) Clamp(path, requested string) (Clamp, error) {
}
ceil, _ := s.Value(decl.ClampedBy)
cdecl, _ := s.schema.Lookup(decl.ClampedBy)
if _, ok := decl.Rank(requested); !ok {
return Clamp{}, fmt.Errorf("%s: %q is not one of %v", path, requested, decl.Enum)
}
want, ok := cdecl.Rank(requested)
if !ok {
return Clamp{}, fmt.Errorf("%s: %q is not one of %v", path, requested, cdecl.Enum)
Expand Down
15 changes: 15 additions & 0 deletions pkgs/mcpx/internal/settings/clamp_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,19 @@ func TestACallerCannotRaiseAClampedSetting(t *testing.T) {
if _, err := set.Clamp("t.dial", "plan"); err == nil {
t.Errorf("an unknown level should be an error, not a silent default")
}
// Ceiling-ladder values outside the dial's own enum are rejected, and
// the error names the dial's values, not the ceiling's (#1641).
for _, bad := range []string{"off", "advise", "ask", "apply"} {
_, err := view.Clamp("t.dial", bad)
Comment on lines +99 to +100

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add the required registry-wide clamp validation guard

The regression test checks only the synthetic t.dial and four hard-coded inputs, leaving current and future registered clamped settings outside the guard. This is a class-wide enum-validation fix, and pkgs/mcpx/internal/AGENTS.md explicitly requires registry/table iteration covering every member. Add a guard that iterates registered settings with ClampedBy, derives each ceiling's values absent from the dial's enum, and asserts that those requests are rejected; retain the focused fixture test for its error-message assertions.

AGENTS.md reference: pkgs/mcpx/internal/AGENTS.md:L11-L12

Useful? React with 👍 / 👎.

if err == nil {
t.Errorf("Clamp(t.dial, %q) should error: not in the dial's enum", bad)
continue
}
if !strings.Contains(err.Error(), "[propose run]") {
t.Errorf("Clamp(t.dial, %q) error should name the dial's values: %v", bad, err)
}
if strings.Contains(err.Error(), "off advise ask") {
t.Errorf("Clamp(t.dial, %q) error should not list the ceiling ladder: %v", bad, err)
}
}
}
Loading