diff --git a/go.mod b/go.mod index 2507b985a..3edec1e30 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/pkg/modules/mcp/inputs.go b/pkg/modules/mcp/inputs.go new file mode 100644 index 000000000..65a431e7a --- /dev/null +++ b/pkg/modules/mcp/inputs.go @@ -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 . + +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 `CreateInput` / `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 `_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 `_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 `_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 +} diff --git a/pkg/modules/mcp/inputs_test.go b/pkg/modules/mcp/inputs_test.go new file mode 100644 index 000000000..5bb66db6e --- /dev/null +++ b/pkg/modules/mcp/inputs_test.go @@ -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 . + +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) +}