From b08d40cba217117d5a2e4a4685ddf3c38bd00ead Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Wed, 28 Oct 2026 14:39:07 +0100 Subject: [PATCH] test: expect the BM management MCP tools - 28 typed management tools pinned in TestBMMCP - body keys are Fillable columns, create inputs require the required columns, material save/update require body, source and url - no seed or me tool, deletes only where the route deletes - every description names its scope and permission --- mcp_test.go | 215 +++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 204 insertions(+), 11 deletions(-) diff --git a/mcp_test.go b/mcp_test.go index 28679ec..b6f8eb3 100644 --- a/mcp_test.go +++ b/mcp_test.go @@ -18,6 +18,7 @@ import ( "git.golem15.com/golem15/summercms/modules/pier" "git.golem15.com/golem15/summercms/modules/surf" bm "git.golem15.com/jakub/sm-bm-plugin" + "git.golem15.com/jakub/sm-bm-plugin/models" ) // mountHost adapts a built surf router to pier.Host for a compile-only @@ -51,6 +52,34 @@ var pinnedTools = []string{ "bm_courses", "bm_dashboard", "bm_deadlines", + "bm_manage_create_class_session", + "bm_manage_create_course", + "bm_manage_create_deadline", + "bm_manage_create_meeting", + "bm_manage_create_reading", + "bm_manage_delete_class_session", + "bm_manage_delete_deadline", + "bm_manage_delete_material", + "bm_manage_delete_reading", + "bm_manage_list_class_sessions", + "bm_manage_list_courses", + "bm_manage_list_deadlines", + "bm_manage_list_materials", + "bm_manage_list_meetings", + "bm_manage_list_readings", + "bm_manage_save_material", + "bm_manage_show_class_session", + "bm_manage_show_course", + "bm_manage_show_deadline", + "bm_manage_show_material", + "bm_manage_show_meeting", + "bm_manage_show_reading", + "bm_manage_update_class_session", + "bm_manage_update_course", + "bm_manage_update_deadline", + "bm_manage_update_material", + "bm_manage_update_meeting", + "bm_manage_update_reading", "bm_mark_reading_owned", "bm_mark_reading_read", "bm_material", @@ -63,16 +92,23 @@ var pinnedTools = []string{ "user_me", } -// studentQueryKeys are the query keys each student handler reads -// (controllers/api: query(r, ...)), by tool name. A tool query field outside -// these would be sent and silently ignored. -var studentQueryKeys = map[string][]string{ - "courses": {"semester"}, - "schedule": {"from", "to"}, - "deadlines": {"from", "to"}, - "readings": {"course"}, - "materials": {"course", "meeting", "tag", "type", "page", "per_page"}, - "material": {"include_body"}, +// queryKeys are the query keys each handler reads, by tool name: the +// student handlers' query(r, ...) and the management record Filters +// (routes.go) plus include_body of the material views. A tool query field +// outside these would be sent and silently ignored. +var queryKeys = map[string][]string{ + "manage_list_courses": {"semester", "form", "code"}, + "manage_list_class_sessions": {"course_id", "meeting_id"}, + "manage_list_readings": {"course_id", "meeting_id", "is_required"}, + "manage_list_deadlines": {"course_id", "kind"}, + "manage_list_materials": {"include_body"}, + "manage_show_material": {"include_body"}, + "courses": {"semester"}, + "schedule": {"from", "to"}, + "deadlines": {"from", "to"}, + "readings": {"course"}, + "materials": {"course", "meeting", "tag", "type", "page", "per_page"}, + "material": {"include_body"}, } // bmTools returns jz.bm's declared tools. @@ -156,7 +192,7 @@ func TestBMMCP(t *testing.T) { t.Errorf("%s.%s: path tag %q not in %s", tool.Name, f.Name, f.Tag.Get("path"), tool.Path) } case f.Tag.Get("query") != "": - if strings.HasPrefix(tool.Path, bm.TokenPrefix+"/") && !slices.Contains(studentQueryKeys[tool.Name], f.Tag.Get("query")) { + if !slices.Contains(queryKeys[tool.Name], f.Tag.Get("query")) { t.Errorf("%s.%s: query key %q is not one the handler reads", tool.Name, f.Name, f.Tag.Get("query")) } default: @@ -167,6 +203,163 @@ func TestBMMCP(t *testing.T) { } } }) + + t.Run("management", func(t *testing.T) { + testManageTools(t, tools) + }) + +} + +// recordModel is what the management record tests read of a model. +type recordModel interface { + Fillable() []string + Rules() map[string]string +} + +// manageTables maps a management path segment to its model (nil for +// materials), its permission code and whether the route can delete. +var manageTables = map[string]struct { + model recordModel + perm string + deletable bool +}{ + "courses": {models.Course{}, "jz.bm.access_courses", false}, + "meetings": {models.Meeting{}, "jz.bm.access_schedule", false}, + "class-sessions": {models.ClassSession{}, "jz.bm.access_schedule", true}, + "readings": {models.Reading{}, "jz.bm.access_courses", true}, + "deadlines": {models.Deadline{}, "jz.bm.access_schedule", true}, + "materials": {nil, "jz.bm.access_materials", true}, +} + +// materialBodyKeys are the keys POST and PUT /materials read (materialInput). +var materialBodyKeys = []string{"course_id", "meeting_id", "title", "type", "body", "source", "url", "published_at", "tags"} + +// bodyFields returns the JSON keys of a tool input's body fields and +// whether each is optional (omitempty). +func bodyFields(input any) map[string]bool { + rt := reflect.TypeOf(input) + for rt.Kind() == reflect.Pointer { + rt = rt.Elem() + } + out := map[string]bool{} + for i := 0; i < rt.NumField(); i++ { + f := rt.Field(i) + if f.Tag.Get("path") != "" || f.Tag.Get("query") != "" { + continue + } + key, opts, _ := strings.Cut(f.Tag.Get("json"), ",") + out[key] = strings.Contains(opts, "omitempty") + } + return out +} + +// requiredColumns are the columns whose rules start with or contain +// required. +func requiredColumns(m recordModel) []string { + var out []string + for col, rule := range m.Rules() { + if slices.Contains(strings.Split(rule, "|"), "required") { + out = append(out, col) + } + } + return out +} + +// testManageTools (D-13, D-14, D-15, D-22) checks the management tools +// against the routes and the models. +func testManageTools(t *testing.T, tools []pact.MCPTool) { + t.Helper() + deletes := map[string]bool{} + manage := 0 + for _, tool := range tools { + if !strings.HasPrefix(tool.Name, "manage_") { + if !strings.HasPrefix(tool.Path, bm.TokenPrefix+"/") { + t.Errorf("%s binds %s, outside the token twins", tool.Name, tool.Path) + } + continue + } + manage++ + if !strings.HasPrefix(tool.Path, bm.ManagePrefix+"/") { + t.Errorf("%s binds %s, outside %s", tool.Name, tool.Path, bm.ManagePrefix) + continue + } + rest := strings.TrimPrefix(tool.Path, bm.ManagePrefix+"/") + segment, _, _ := strings.Cut(rest, "/") + table, ok := manageTables[segment] + if !ok { + t.Errorf("%s binds %s, not a managed table (seed and me are not tools)", tool.Name, tool.Path) + continue + } + if !strings.Contains(tool.Description, "Needs scope") || !strings.Contains(tool.Description, "jz.bm.access_") { + t.Errorf("%s: description names no scope or permission: %q", tool.Name, tool.Description) + } + if !strings.Contains(tool.Description, table.perm) { + t.Errorf("%s: description does not name %s", tool.Name, table.perm) + } + if tool.Method == http.MethodDelete { + deletes[segment] = true + if !table.deletable { + t.Errorf("%s deletes %s, which the route cannot", tool.Name, segment) + } + if !strings.Contains(tool.Description, "cannot be undone") { + t.Errorf("%s: description does not say the delete cannot be undone", tool.Name) + } + } + fields := bodyFields(tool.Input) + if segment == "materials" { + if !strings.Contains(tool.Description, bm.MCPMaterialPrivacy) { + t.Errorf("%s: description lacks the material privacy sentence", tool.Name) + } + for key := range fields { + if !slices.Contains(materialBodyKeys, key) { + t.Errorf("%s: body key %q is not a material key", tool.Name, key) + } + } + if tool.Method == http.MethodPost || tool.Method == http.MethodPut { + for _, key := range []string{"title", "type", "body", "source", "url"} { + if optional, ok := fields[key]; !ok || optional { + t.Errorf("%s: %s must be a required field (present %v)", tool.Name, key, ok) + } + } + if !strings.Contains(tool.Description, "replaced as sent") { + t.Errorf("%s: description does not say body, source and url are replaced as sent", tool.Name) + } + } + continue + } + fillable := table.model.Fillable() + for key := range fields { + if !slices.Contains(fillable, key) { + t.Errorf("%s: body key %q is not a Fillable column of %s", tool.Name, key, segment) + } + } + switch tool.Method { + case http.MethodPost: + for _, col := range fillable { + if _, ok := fields[col]; !ok { + t.Errorf("%s: Fillable column %s has no field", tool.Name, col) + } + } + for _, col := range requiredColumns(table.model) { + if optional, ok := fields[col]; !ok || optional { + t.Errorf("%s: required column %s is not a required field", tool.Name, col) + } + } + case http.MethodPatch: + for _, col := range fillable { + if optional, ok := fields[col]; !ok || !optional { + t.Errorf("%s: column %s must be an optional field (present %v)", tool.Name, col, ok) + } + } + } + } + if manage != 28 { + t.Errorf("%d management tools, want 28", manage) + } + want := map[string]bool{"class-sessions": true, "readings": true, "deadlines": true, "materials": true} + if !reflect.DeepEqual(deletes, want) { + t.Errorf("delete tools for %v, want %v", deletes, want) + } } // studentRoutes are the routes mountStudent registers, in order.