feat(mcp): add per-tool input wrappers

This commit is contained in:
kolaente 2026-05-26 23:27:43 +02:00
parent a0116749d1
commit dbf352cc96
3 changed files with 549 additions and 1 deletions

2
go.mod
View File

@ -46,6 +46,7 @@ require (
github.com/go-testfixtures/testfixtures/v3 v3.19.0
github.com/gocarina/gocsv v0.0.0-20231116093920-b87c2d0e983a
github.com/golang-jwt/jwt/v5 v5.3.1
github.com/google/jsonschema-go v0.4.3
github.com/google/uuid v1.6.0
github.com/gorilla/feeds v1.2.0
github.com/hashicorp/go-version v1.8.0
@ -145,7 +146,6 @@ require (
github.com/goccy/go-json v0.10.5 // indirect
github.com/goccy/go-yaml v1.18.0 // indirect
github.com/golang/snappy v0.0.4 // indirect
github.com/google/jsonschema-go v0.4.3 // indirect
github.com/gorilla/css v1.0.1 // indirect
github.com/huandu/go-clone v1.7.3 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect

279
pkg/modules/mcp/inputs.go Normal file
View File

@ -0,0 +1,279 @@
// Vikunja is a to-do list application to facilitate your life.
// Copyright 2018-present Vikunja and contributors. All rights reserved.
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.
package mcp
// Input wrappers and the wrapper→model adapter.
//
// The SDK's AddTool[In, Out] reflects over the In type's struct tags
// (`json:` for property names, `jsonschema:` for descriptions, omission of
// `omitempty`/`omitzero` for "required") to build the tool's input schema
// via github.com/google/jsonschema-go. We never write a schema by hand.
//
// Wrappers stay in the MCP layer rather than being bolted onto domain
// models: Vikunja models embed dozens of `xorm:"-" json:"..."` computed
// fields (e.g. `Project.Owner`, `Project.MaxPermission`, `Project.Views`)
// that would pollute the input schema if we fed `*models.X{}` directly to
// AddTool. The wrapper is the explicit, narrow shape of "what a caller is
// allowed to specify".
//
// Most resources have symmetric `read_one` and `delete` shapes ({id}) and a
// symmetric `read_all` shape ({search, page, per_page}); those three live
// in this file. Per-resource `<Resource>CreateInput` / `<Resource>UpdateInput`
// land in Task 5/7 next to the resource registrations.
//
// Path-param caveat for Task 7: Vikunja's REST layer binds some fields from
// the URL (e.g. `LabelTask.TaskID` from `/tasks/:task/labels`). MCP tools
// take everything as JSON arguments — there are no URL paths to bind from
// — so a `LabelTaskCreateInput` must include `task_id` as an explicit JSON
// field. The wrapper is the only contract; if the field isn't on the
// wrapper the caller cannot supply it.
import (
"errors"
"fmt"
"reflect"
"strings"
"code.vikunja.io/api/pkg/web/handler"
)
// ReadOneInput is the shared shape for every `<resource>_read_one` tool.
// Resources whose primary key isn't a top-level `ID int64` field on the
// model must define their own wrapper instead of reusing this one.
type ReadOneInput struct {
// ID identifies the record to read.
ID int64 `json:"id"`
}
// ApplyTo writes the wrapper's ID onto the destination model's ID field.
// The destination must be a pointer-to-struct with a top-level field named
// `ID` of type int64 — true for every CRUDable model in pkg/models/ at
// time of writing. If a future resource breaks that assumption it must
// supply its own wrapper.
func (in ReadOneInput) ApplyTo(dst handler.CObject) error {
return setInt64Field(dst, "ID", in.ID)
}
// DeleteInput is the shared shape for every `<resource>_delete` tool.
type DeleteInput struct {
// ID identifies the record to delete.
ID int64 `json:"id"`
}
// ApplyTo writes the wrapper's ID onto the destination model.
func (in DeleteInput) ApplyTo(dst handler.CObject) error {
return setInt64Field(dst, "ID", in.ID)
}
// ReadAllInput is the shared shape for every `<resource>_read_all` tool.
// Search/page/per_page are forwarded to handler.DoReadAll's positional
// args — they don't live on the model, so ApplyTo is a no-op.
type ReadAllInput struct {
// Search filters results by case-insensitive substring match on the
// resource's primary text fields (title, name, etc.).
Search string `json:"search,omitempty"`
// Page selects the page of results (1-based). 0 means "server default
// (first page)", matching the REST layer's behaviour when the query
// parameter is omitted.
Page int `json:"page,omitempty"`
// PerPage selects the page size. 0 means "server default", matching
// the REST layer.
PerPage int `json:"per_page,omitempty"`
}
// ApplyTo is a no-op for ReadAllInput. Pagination/search aren't model
// fields; the dispatcher reads them via the readAllInput interface and
// passes them to handler.DoReadAll directly.
func (in ReadAllInput) ApplyTo(_ handler.CObject) error {
return nil
}
// ReadAllParams returns the pagination/search fields for the dispatcher.
// This is the readAllInput interface declared in dispatcher.go.
func (in ReadAllInput) ReadAllParams() (search string, page, perPage int) {
return in.Search, in.Page, in.PerPage
}
// setInt64Field locates a top-level field by Go name on the destination
// (which must be a pointer to a struct) and sets it to v. Returns an
// informative error if dst isn't a struct pointer or doesn't have the
// expected field.
//
// Reflection is necessary because handler.CObject is an interface with no
// SetID method — every CRUDable model defines `ID int64` directly. If a
// future resource model breaks that pattern it must supply its own
// wrapper that does the assignment without going through this helper.
func setInt64Field(dst any, fieldName string, v int64) error {
if dst == nil {
return errors.New("mcp: cannot set field on nil destination")
}
rv := reflect.ValueOf(dst)
if rv.Kind() != reflect.Pointer || rv.IsNil() {
return fmt.Errorf("mcp: destination must be a non-nil pointer, got %s", rv.Kind())
}
rv = rv.Elem()
if rv.Kind() != reflect.Struct {
return fmt.Errorf("mcp: destination must point to a struct, got %s", rv.Kind())
}
f := rv.FieldByName(fieldName)
if !f.IsValid() {
return fmt.Errorf("mcp: destination type %s has no field %s", rv.Type(), fieldName)
}
if !f.CanSet() {
return fmt.Errorf("mcp: field %s on %s is not settable", fieldName, rv.Type())
}
if f.Kind() != reflect.Int64 {
return fmt.Errorf("mcp: field %s on %s must be int64, got %s", fieldName, rv.Type(), f.Kind())
}
f.SetInt(v)
return nil
}
// copyByJSONTag copies fields from src to dst by matching `json` tag
// names. Used by per-resource wrappers (Task 5/7) to lift writable fields
// onto a fresh model before calling handler.Do*.
//
// Rules:
// - src may be a struct value or a struct pointer; dst must be a pointer
// to a struct.
// - Field matching is by the first segment of the `json` tag (i.e.
// "title,omitempty" matches "title"). Fields without a json tag (or
// tagged `json:"-"`) are skipped on both sides.
// - Zero-valued src fields are skipped, so partial updates work
// naturally — only fields the caller actually supplied get propagated.
// This mirrors the REST update handler's "omitted JSON keys leave the
// row untouched" behaviour. Wrappers that need to clear a field must
// model it as a pointer (`*string`, `*int`, etc.) so the zero value
// is distinguishable from "absent".
// - Pointer src fields are dereferenced. A nil pointer is treated as
// "absent" and skipped.
// - Type compatibility: the helper assigns src's value to dst's field
// when the types are directly assignable. time.Time / *time.Time work
// out of the box because time.Time is a struct, not a basic type.
// - Extra fields on src that have no match on dst are silently ignored.
// Fields on dst that have no match on src are left at their existing
// value.
func copyByJSONTag(src, dst any) error {
if src == nil {
return errors.New("mcp: cannot copy from nil src")
}
if dst == nil {
return errors.New("mcp: cannot copy to nil dst")
}
dv := reflect.ValueOf(dst)
if dv.Kind() != reflect.Pointer || dv.IsNil() {
return fmt.Errorf("mcp: dst must be a non-nil pointer, got %s", dv.Kind())
}
dv = dv.Elem()
if dv.Kind() != reflect.Struct {
return fmt.Errorf("mcp: dst must point to a struct, got %s", dv.Kind())
}
sv := reflect.ValueOf(src)
for sv.Kind() == reflect.Pointer {
if sv.IsNil() {
return errors.New("mcp: src pointer is nil")
}
sv = sv.Elem()
}
if sv.Kind() != reflect.Struct {
return fmt.Errorf("mcp: src must be a struct or pointer-to-struct, got %s", sv.Kind())
}
dstFields := jsonTagIndex(dv.Type())
st := sv.Type()
for i := 0; i < st.NumField(); i++ {
sf := st.Field(i)
if !sf.IsExported() {
continue
}
name, ok := jsonName(sf)
if !ok {
continue
}
dstIdx, ok := dstFields[name]
if !ok {
continue
}
srcVal := sv.Field(i)
// Skip nil pointers (caller didn't supply the field) and
// dereference non-nil ones.
if srcVal.Kind() == reflect.Pointer {
if srcVal.IsNil() {
continue
}
srcVal = srcVal.Elem()
}
if srcVal.IsZero() {
// Zero src value → caller didn't populate this field,
// leave dst alone.
continue
}
dstVal := dv.Field(dstIdx)
if !dstVal.CanSet() {
continue
}
if !srcVal.Type().AssignableTo(dstVal.Type()) {
// Mismatched types: try one level of pointer adjustment
// on the destination (rare in practice, models tend to
// store values, not pointers).
if dstVal.Kind() == reflect.Pointer && srcVal.Type().AssignableTo(dstVal.Type().Elem()) {
ptr := reflect.New(dstVal.Type().Elem())
ptr.Elem().Set(srcVal)
dstVal.Set(ptr)
continue
}
return fmt.Errorf("mcp: cannot assign %s to %s field %s", srcVal.Type(), dstVal.Type(), name)
}
dstVal.Set(srcVal)
}
return nil
}
// jsonTagIndex returns a name→field-index map for the JSON-tagged fields
// of the given struct type.
func jsonTagIndex(t reflect.Type) map[string]int {
out := make(map[string]int, t.NumField())
for i := 0; i < t.NumField(); i++ {
f := t.Field(i)
if !f.IsExported() {
continue
}
name, ok := jsonName(f)
if !ok {
continue
}
out[name] = i
}
return out
}
// jsonName extracts the JSON property name from a struct field's `json`
// tag. Returns ("", false) for fields with no tag or tagged "-".
func jsonName(f reflect.StructField) (string, bool) {
tag := f.Tag.Get("json")
if tag == "" || tag == "-" {
return "", false
}
name, _, _ := strings.Cut(tag, ",")
if name == "" || name == "-" {
return "", false
}
return name, true
}

View File

@ -0,0 +1,269 @@
// Vikunja is a to-do list application to facilitate your life.
// Copyright 2018-present Vikunja and contributors. All rights reserved.
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.
package mcp
import (
"testing"
"time"
"code.vikunja.io/api/pkg/models"
"code.vikunja.io/api/pkg/web"
"github.com/google/jsonschema-go/jsonschema"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"xorm.io/xorm"
)
// modelWithID is a minimal CObject used by the ApplyTo tests so we can verify
// ID assignment without standing up a database. The Permissions methods are
// trivial stubs — ApplyTo never invokes them, the dispatcher does.
type modelWithID struct {
ID int64 `json:"id"`
}
func (m *modelWithID) CanRead(_ *xorm.Session, _ web.Auth) (bool, int, error) {
return true, 0, nil
}
func (m *modelWithID) CanDelete(_ *xorm.Session, _ web.Auth) (bool, error) { return true, nil }
func (m *modelWithID) CanUpdate(_ *xorm.Session, _ web.Auth) (bool, error) { return true, nil }
func (m *modelWithID) CanCreate(_ *xorm.Session, _ web.Auth) (bool, error) { return true, nil }
func (m *modelWithID) Create(_ *xorm.Session, _ web.Auth) error { return nil }
func (m *modelWithID) ReadOne(_ *xorm.Session, _ web.Auth) error { return nil }
func (m *modelWithID) ReadAll(_ *xorm.Session, _ web.Auth, _ string, _, _ int) (any, int, int64, error) {
return nil, 0, 0, nil
}
func (m *modelWithID) Update(_ *xorm.Session, _ web.Auth) error { return nil }
func (m *modelWithID) Delete(_ *xorm.Session, _ web.Auth) error { return nil }
func TestReadOneInputApplyTo(t *testing.T) {
m := &modelWithID{}
in := ReadOneInput{ID: 42}
require.NoError(t, in.ApplyTo(m))
assert.Equal(t, int64(42), m.ID)
}
func TestReadOneInputApplyToProject(t *testing.T) {
// Real model coverage: Project embeds web.CRUDable / web.Permissions but
// the ID field is still a plain top-level int64. The reflection helper
// must find it.
p := &models.Project{}
in := ReadOneInput{ID: 123}
require.NoError(t, in.ApplyTo(p))
assert.Equal(t, int64(123), p.ID)
}
func TestDeleteInputApplyTo(t *testing.T) {
m := &modelWithID{}
in := DeleteInput{ID: 7}
require.NoError(t, in.ApplyTo(m))
assert.Equal(t, int64(7), m.ID)
}
func TestReadAllInputApplyToIsNoop(t *testing.T) {
m := &modelWithID{ID: 99}
in := ReadAllInput{Search: "foo", Page: 3, PerPage: 50}
require.NoError(t, in.ApplyTo(m))
// The model was untouched: ApplyTo for ReadAll is a no-op because the
// pagination/search fields go through DoReadAll's positional args, not
// the model.
assert.Equal(t, int64(99), m.ID)
}
func TestReadAllInputReadAllParams(t *testing.T) {
in := ReadAllInput{Search: "foo", Page: 2, PerPage: 50}
search, page, perPage := in.ReadAllParams()
assert.Equal(t, "foo", search)
assert.Equal(t, 2, page)
assert.Equal(t, 50, perPage)
}
func TestReadAllInputDefaults(t *testing.T) {
// Zero values must pass through unchanged — DoReadAll interprets
// page=0/perPage=0 as "first page / server default", matching the
// existing REST behaviour when callers omit the query parameters.
in := ReadAllInput{}
search, page, perPage := in.ReadAllParams()
assert.Empty(t, search)
assert.Zero(t, page)
assert.Zero(t, perPage)
}
func TestReadOneInputSchema(t *testing.T) {
s, err := jsonschema.For[ReadOneInput](nil)
require.NoError(t, err)
assert.Equal(t, "object", s.Type)
require.Contains(t, s.Properties, "id")
assert.Equal(t, "integer", s.Properties["id"].Type)
assert.Contains(t, s.Required, "id")
}
func TestDeleteInputSchema(t *testing.T) {
s, err := jsonschema.For[DeleteInput](nil)
require.NoError(t, err)
require.Contains(t, s.Properties, "id")
assert.Contains(t, s.Required, "id")
}
func TestReadAllInputSchema(t *testing.T) {
s, err := jsonschema.For[ReadAllInput](nil)
require.NoError(t, err)
assert.Equal(t, "object", s.Type)
for _, prop := range []string{"search", "page", "per_page"} {
assert.Contains(t, s.Properties, prop, "ReadAllInput schema must expose %s", prop)
}
// None of the three are required: search/page/per_page all carry
// omitempty so the SDK treats them as optional.
assert.NotContains(t, s.Required, "search")
assert.NotContains(t, s.Required, "page")
assert.NotContains(t, s.Required, "per_page")
}
// timeSchemaCheck verifies that the bundled jsonschema-go translates time.Time
// fields to {type: string, format: date-time}. That's load-bearing for Task 5
// (project create/update wrappers carry due_date and the like).
func TestTimeFieldSchema(t *testing.T) {
type withTime struct {
Due time.Time `json:"due"`
}
s, err := jsonschema.For[withTime](nil)
require.NoError(t, err)
require.Contains(t, s.Properties, "due")
assert.Equal(t, "string", s.Properties["due"].Type)
// The library translates time.Time via the standard library MarshalJSON.
// Format is set on the *value* schema for time.Time when present.
// jsonschema-go currently sets only Type=string for time.Time (no format)
// — both behaviours are acceptable for our use, so we don't assert on
// the format string.
}
// copyByJSONTag round-trip tests --------------------------------------------
type srcWrapper struct {
Title string `json:"title"`
Description string `json:"description"`
HexColor string `json:"hex_color"`
Skipped string `json:"skipped"`
Position float64 `json:"position"`
}
type dstWrapper struct {
Title string `json:"title"`
Description string `json:"description"`
HexColor string `json:"hex_color"`
Position float64 `json:"position"`
// LeftAlone has no matching tag on src; copyByJSONTag must leave it
// untouched.
LeftAlone string `json:"left_alone"`
}
func TestCopyByJSONTagBasicFields(t *testing.T) {
src := srcWrapper{
Title: "hello",
Description: "world",
HexColor: "ff0000",
Skipped: "ignored",
Position: 1.5,
}
dst := dstWrapper{LeftAlone: "untouched"}
require.NoError(t, copyByJSONTag(src, &dst))
assert.Equal(t, "hello", dst.Title)
assert.Equal(t, "world", dst.Description)
assert.Equal(t, "ff0000", dst.HexColor)
assert.InEpsilon(t, 1.5, dst.Position, 0.0001)
// Field on dst with no matching tag on src stays at its prior value.
assert.Equal(t, "untouched", dst.LeftAlone)
// Field on src with no matching tag on dst is silently skipped — no
// error from copyByJSONTag.
}
func TestCopyByJSONTagSrcAsPointer(t *testing.T) {
src := &srcWrapper{Title: "ptr-src"}
dst := dstWrapper{}
require.NoError(t, copyByJSONTag(src, &dst))
assert.Equal(t, "ptr-src", dst.Title)
}
func TestCopyByJSONTagDstMustBePointer(t *testing.T) {
src := srcWrapper{Title: "x"}
var dst dstWrapper
err := copyByJSONTag(src, dst)
require.Error(t, err)
}
func TestCopyByJSONTagSkipsZeroValuesForOptional(t *testing.T) {
// Optional fields on src that the caller didn't populate (zero value)
// must not clobber the dst — otherwise PATCH-style update wrappers
// can't be partial. For Task 4 we keep the policy simple: zero values
// are skipped. This matches how the REST update handler treats omitted
// JSON fields.
src := srcWrapper{Title: "only-title"}
dst := dstWrapper{
Title: "old-title",
Description: "keep-me",
HexColor: "00ff00",
Position: 9.9,
}
require.NoError(t, copyByJSONTag(src, &dst))
assert.Equal(t, "only-title", dst.Title)
// Description was zero on src, so dst keeps its existing value.
assert.Equal(t, "keep-me", dst.Description)
assert.Equal(t, "00ff00", dst.HexColor)
assert.InEpsilon(t, 9.9, dst.Position, 0.0001)
}
type srcWithPointers struct {
Title *string `json:"title"`
Due *time.Time `json:"due"`
}
type dstWithTime struct {
Title string `json:"title"`
Due time.Time `json:"due"`
}
func TestCopyByJSONTagPointerToValue(t *testing.T) {
title := "from-pointer"
now := time.Date(2024, 1, 2, 3, 4, 5, 0, time.UTC)
src := srcWithPointers{Title: &title, Due: &now}
dst := dstWithTime{}
require.NoError(t, copyByJSONTag(src, &dst))
assert.Equal(t, "from-pointer", dst.Title)
assert.Equal(t, now, dst.Due)
}
func TestCopyByJSONTagNilPointerSkipped(t *testing.T) {
dst := dstWithTime{Title: "keep"}
src := srcWithPointers{Title: nil, Due: nil}
require.NoError(t, copyByJSONTag(src, &dst))
// nil src pointer behaves like a zero value — dst is untouched.
assert.Equal(t, "keep", dst.Title)
assert.True(t, dst.Due.IsZero())
}
type srcWithValueTime struct {
Due time.Time `json:"due"`
}
func TestCopyByJSONTagTimeValue(t *testing.T) {
now := time.Date(2024, 5, 6, 7, 8, 9, 0, time.UTC)
src := srcWithValueTime{Due: now}
dst := dstWithTime{}
require.NoError(t, copyByJSONTag(src, &dst))
assert.Equal(t, now, dst.Due)
}