fix(notifications): refuse a level the update scope does not accept

cmd/audit_meta_descriptions/main.go

@@ -109,19 +109,29 @@ func main() {
 	uncovered := flag.Bool("uncovered", false, "name the enumeration lines the extraction rule refused instead of only counting them")
 	flag.Parse()
 
-	// os.Exit lives here, not in run: run holds the stub client's deferred
-	// cleanup, and an exit inside it would skip that defer.
+	// os.Exit lives here, not in run: run is what the tests drive for its exit
+	// code, and an exit inside it would take the test binary with it.
 	os.Exit(run(*check, *uncovered))
 }
 
 // run performs the audit and returns the process exit code.
 func run(check, uncovered bool) int {
+	findings, lines, refused := auditServedSurface()
+	return report(findings, lines, refused, check, uncovered)
+}
+
+// auditServedSurface builds the catalog, lists the meta surface from it, and
+// returns what the comparison found. It is separate from [run] so that a test
+// can assert one rule over the real surface without reassembling this setup:
+// a test that builds its own copy of the thing under test is testing the copy.
+// The stub client's cleanup is deferred here, and the returned values hold
+// nothing of it, so the caller may exit the process on what it gets back.
+func auditServedSurface() (findings []finding, lines int, refused []skipped) {
 	client, cleanup := mcpsurface.NewStubClient()
 	defer cleanup()
 
 	catalog := cmdutil.Must(tools.BuildActionCatalog(client, tools.ActionCatalogOptions{Enterprise: true, IncludeMCP: true}))
-	findings, lines, refused := audit(metaTools(client), catalog)
-	return report(findings, lines, refused, check, uncovered)
+	return audit(metaTools(client), catalog)
 }
 
 // metaTools lists the meta surface at the widest tier over a real tools/list

cmd/audit_meta_descriptions/main_test.go

@@ -1,7 +1,8 @@
 // Package main tests the meta description auditor: the extraction rule for
 // both blocks a description enumerates parameters in, the comparison against
-// the routes' input schemas, the report and its exit codes, and one full run
-// over the served surface, which is the CI gate's own assertion.
+// the routes' input schemas, the report and its exit codes, and the served
+// surface itself: once as the full run the CI gate makes, and once as the value
+// rule alone, which names the action and the value a line offers wrongly.
 package main
 
 import (
@@ -794,3 +795,30 @@ func TestRun_ServedSurface_IsClean(t *testing.T) {
 		t.Errorf("stdout = %q, want the all-clear summary with nothing refused", out.String())
 	}
 }
+
+// TestAuditServedSurface_OffersNoValueTheActionItNamesRejects holds one rule of
+// the gate on its own: a served line that names a single action and spells a
+// value set for one of its parameters offers only values that action's own
+// input schema accepts. The values are read back from the schema rather than
+// listed here, so the test states the rule instead of pinning today's answer.
+//
+// It is the value rule at per-action grain, which is the grain a shared list
+// hides. gitlab_user's Notifications block offered "global" for
+// notification_global_update long after that action stopped accepting it: the
+// account-wide scope publishes one level fewer than its project and group
+// siblings, client-go refuses the extra one before it builds a request, and the
+// pooled union of the group would have admitted it. [TestRun_ServedSurface_IsClean]
+// covers this too, as one number among every rule's findings; this one names the
+// action and the value, which is what a reader needs when it regresses.
+func TestAuditServedSurface_OffersNoValueTheActionItNamesRejects(t *testing.T) {
+	findings, lines, _ := auditServedSurface()
+	if lines == 0 {
+		t.Fatal("audit read no description lines, so it asserted nothing")
+	}
+	for _, f := range findings {
+		if f.kind != kindEnumValue && f.kind != kindDocValue {
+			continue
+		}
+		t.Errorf("%s offers %s, which the schema of the action that line names rejects: %s", f.tool, f.detail, f.line)
+	}
+}

internal/tools/notifications/action_specs.go

@@ -1,6 +1,8 @@
 package notifications
 
 import (
+	"strings"
+
 	gitlabclient "github.com/jmrplens/gitlab-mcp-server/v3/internal/gitlab"
 	"github.com/jmrplens/gitlab-mcp-server/v3/internal/toolutil"
 )
@@ -87,14 +89,54 @@ func notificationOptions(name, individualTool string) toolutil.ActionSpecOptions
 		options.IndividualTool.Description = meta.description
 	}
 	switch name {
-	case actionGlobalUpdate, actionProjectUpdate, actionGroupUpdate:
-		options.InputSchemaOverrides = []toolutil.InputSchemaOverride{
-			toolutil.SchemaPropertyOverride("level", map[string]any{"enum": []any{"disabled", "participating", "watch", "global", "mention", "custom"}}),
-		}
+	case actionGlobalUpdate:
+		options.InputSchemaOverrides = levelSchemaOverrides(globalLevels)
+	case actionProjectUpdate, actionGroupUpdate:
+		options.InputSchemaOverrides = levelSchemaOverrides(scopedLevels)
 	}
 	return options
 }
 
+// levelSchemaOverrides publishes the levels one update scope accepts, as both
+// the enum a validating surface checks and the description every surface shows.
+//
+// Both halves are needed, for different readers. The enum is advisory on the
+// default dynamic surface and on the opaque meta surface, which validate
+// parameter names and requiredness and nothing else, so the description is the
+// copy a model actually reads; the individual surface is the only one where the
+// enum itself refuses a value. And the description cannot live on the embedded
+// eventFields, which all three update inputs share and which therefore cannot
+// say that the account-wide scope accepts one level fewer.
+func levelSchemaOverrides(levels []string) []toolutil.InputSchemaOverride {
+	return []toolutil.InputSchemaOverride{
+		toolutil.SchemaEnumOverride("level", levels...),
+		toolutil.SchemaPropertyOverride("level", map[string]any{"description": levelDescription(levels)}),
+	}
+}
+
+// levelDescription renders the `level` parameter's published description from
+// the list one scope accepts, so the prose and the enum beside it are one list
+// rather than two copies of one.
+func levelDescription(levels []string) string {
+	return "Notification level: " + strings.Join(levels, ", ")
+}
+
+// updateUsage renders one update action's Usage sentence around the levels its
+// own scope accepts, between the head that says which scope is being written
+// and the tail that names the flags.
+//
+// It exists because the Usage sentence is the third place one scope's level
+// list is published, after the enum and the parameter description that
+// [levelSchemaOverrides] already renders from one list, and it was the copy
+// nothing compares: the meta surface's "Action guidance" block is stripped
+// before cmd/audit_meta_descriptions reads a description, so a level spelled
+// here that the action's enum refuses would reach a model unchallenged.
+// Derived, the sentence cannot say more than the enum allows.
+func updateUsage(head string, levels []string, flags string) string {
+	return head + " level (" + strings.Join(levels, ", ") + "), notification_email, and " +
+		flags + " when level is custom. Only the fields you pass are changed."
+}
+
 // notificationActionMetaEntry is the discovery metadata for one
 // notification settings action.
 type notificationActionMetaEntry struct {
@@ -116,7 +158,10 @@ var notificationActionMeta = map[string]notificationActionMetaEntry{
 		description: "Get the authenticated user's global notification settings. Returns: the global notification level, notification email, and the per-event flags (issue, merge request, pipeline, note, and epic events) when the level is custom. See also: gitlab_notification_global_update, gitlab_notification_project_get, gitlab_notification_group_get.",
 	},
 	actionGlobalUpdate: {
-		usage:       "Update the authenticated user's account-wide (global) notification settings. Set level (disabled, participating, watch, global, mention, custom), notification_email, and individual event flags when level is custom. Only the fields you pass are changed.",
+		usage: updateUsage(
+			"Update the authenticated user's account-wide (global) notification settings. Set",
+			globalLevels, "individual event flags",
+		),
 		aliases:     []string{"update global notification settings", "set my notification level", "change default notification email"},
 		related:     []string{canonicalID(actionGlobalGet), canonicalID(actionProjectUpdate), canonicalID(actionGroupUpdate)},
 		description: "Update the authenticated user's global notification settings. Returns: the updated global notification level, notification email, and per-event flags. See also: gitlab_notification_global_get, gitlab_notification_project_update, gitlab_notification_group_update.",
@@ -128,7 +173,10 @@ var notificationActionMeta = map[string]notificationActionMetaEntry{
 		description: "Get the authenticated user's notification settings for a project. Returns: the project notification level, notification email, and per-event flags when the level is custom. See also: gitlab_notification_project_update, gitlab_notification_global_get, gitlab_notification_group_get.",
 	},
 	actionProjectUpdate: {
-		usage:       "Update the authenticated user's notification settings for one project. Pass project_id plus level (disabled, participating, watch, global, mention, custom), notification_email, and event flags when level is custom. Only the fields you pass are changed.",
+		usage: updateUsage(
+			"Update the authenticated user's notification settings for one project. Pass project_id plus",
+			scopedLevels, "event flags",
+		),
 		aliases:     []string{"update project notification settings", "set notification level for project", "override notifications for project"},
 		related:     []string{canonicalID(actionProjectGet), canonicalID(actionGlobalUpdate), canonicalID(actionGroupUpdate)},
 		description: "Update the authenticated user's notification settings for a project. Returns: the updated project notification level, notification email, and per-event flags. See also: gitlab_notification_project_get, gitlab_notification_global_update, gitlab_notification_group_update.",
@@ -140,7 +188,10 @@ var notificationActionMeta = map[string]notificationActionMetaEntry{
 		description: "Get the authenticated user's notification settings for a group. Returns: the group notification level, notification email, and per-event flags when the level is custom. See also: gitlab_notification_group_update, gitlab_notification_global_get, gitlab_notification_project_get.",
 	},
 	actionGroupUpdate: {
-		usage:       "Update the authenticated user's notification settings for one group. Pass group_id plus level (disabled, participating, watch, global, mention, custom), notification_email, and event flags when level is custom. Only the fields you pass are changed.",
+		usage: updateUsage(
+			"Update the authenticated user's notification settings for one group. Pass group_id plus",
+			scopedLevels, "event flags",
+		),
 		aliases:     []string{"update group notification settings", "set notification level for group", "override notifications for group"},
 		related:     []string{canonicalID(actionGroupGet), canonicalID(actionGlobalUpdate), canonicalID(actionProjectUpdate)},
 		description: "Update the authenticated user's notification settings for a group. Returns: the updated group notification level, notification email, and per-event flags. See also: gitlab_notification_group_get, gitlab_notification_global_update, gitlab_notification_project_update.",

internal/tools/notifications/action_specs_test.go

@@ -5,6 +5,8 @@ package notifications
 import (
 	"net/http"
 	"regexp"
+	"slices"
+	"strings"
 	"testing"
 
 	"github.com/jmrplens/gitlab-mcp-server/v3/internal/testutil"
@@ -92,6 +94,83 @@ func TestFormatMarkdownString_HintsNameActionsTheCatalogHolds(t *testing.T) {
 	}
 }
 
+// TestActionSpecs_Level_PublishesTheScopesOwnListEverywhere asserts that each
+// update action advertises the levels its own scope takes, in all three places
+// a model can read them: the enum the individual surface validates against, the
+// description every surface renders, and the Usage sentence discovery returns.
+//
+// All three actions used to share one six-value enum, which put "global" on the
+// account-wide one. client-go refuses that value in UpdateGlobalSettings before
+// it builds a request, so a model picking it off the published list was
+// guaranteed an error, and the 400 hint the same handler returned offered it
+// the very list it had just failed with. The Usage sentence is checked here
+// rather than assembled in the source, so the prose stays readable where it is
+// written and still cannot drift from the enum beside it.
+func TestActionSpecs_Level_PublishesTheScopesOwnListEverywhere(t *testing.T) {
+	client := testutil.NewTestClient(t, http.HandlerFunc(func(http.ResponseWriter, *http.Request) {
+		t.Error("building the specs should reach no GitLab")
+	}))
+	want := map[string][]string{
+		actionGlobalUpdate:  globalLevels,
+		actionProjectUpdate: scopedLevels,
+		actionGroupUpdate:   scopedLevels,
+	}
+
+	for _, spec := range ActionSpecs(client) {
+		levels, isUpdate := want[spec.Name]
+		if !isUpdate {
+			continue
+		}
+		t.Run(spec.Name, func(t *testing.T) {
+			level := levelProperty(t, spec.Route.InputSchema)
+
+			if got := enumStrings(t, level); !slices.Equal(got, levels) {
+				t.Errorf("enum = %v, want %v", got, levels)
+			}
+			if description, _ := level["description"].(string); description != levelDescription(levels) {
+				t.Errorf("description = %q, want %q", description, levelDescription(levels))
+			}
+			if phrase := "level (" + strings.Join(levels, ", ") + ")"; !strings.Contains(spec.Usage, phrase) {
+				t.Errorf("Usage does not name %q:\n%s", phrase, spec.Usage)
+			}
+		})
+	}
+}
+
+// enumStrings returns the enum a published property carries, refusing anything
+// that is not a list of strings.
+func enumStrings(t *testing.T, property map[string]any) []string {
+	t.Helper()
+	enum, ok := property["enum"].([]any)
+	if !ok {
+		t.Fatalf("property publishes no enum, got %#v", property["enum"])
+	}
+	values := make([]string, 0, len(enum))
+	for _, value := range enum {
+		name, isString := value.(string)
+		if !isString {
+			t.Fatalf("enum holds %#v, want strings", value)
+		}
+		values = append(values, name)
+	}
+	return values
+}
+
+// levelProperty returns the `level` property of an update action's published
+// input schema, which is where both halves of the override land.
+func levelProperty(t *testing.T, schema map[string]any) map[string]any {
+	t.Helper()
+	properties, ok := schema["properties"].(map[string]any)
+	if !ok {
+		t.Fatalf("input schema has no properties: %#v", schema)
+	}
+	level, ok := properties["level"].(map[string]any)
+	if !ok {
+		t.Fatalf("input schema has no level property: %#v", properties)
+	}
+	return level
+}
+
 // TestCanonicalID_QualifiesEverySpecName asserts that a spec name and the ID a
 // caller passes are two different strings, and that the second is the first
 // under this package's catalog domain.