feat(mcp): add typed schema param to pad_collection.create (TASK-1335) (#483)

* feat(mcp): add typed schema param to pad_collection.create (TASK-1335)

TASK-1334 shipped the CLI --schema flag for BUG-1284. This wires the
same capability through the MCP surface so agents calling pad_collection
get a structured object parameter rather than a stringified DSL —
matching how every other Pad-native data structure (items, comments,
etc.) flows through MCP.

Three coordinated changes:

  1. internal/mcp/catalog.go — add "object" Type to ParamDef, mapped to
     mcp-go's WithObject. Generic addition; the schema param is the first
     consumer.

  2. internal/mcp/catalog_collection.go — declare schema (object) param
     on pad_collection.create alongside the existing fields (string)
     param. Tool description now points agents at schema for
     terminal_options, defaults, computed fields, suffixes, and relation
     collections (everything the DSL cannot express), and notes the
     fields/schema mutual exclusion.

  3a. internal/mcp/dispatch.go — BuildCLIArgs now JSON-encodes
      non-string structured values via a new encodeFlagValue helper. The
      ExecDispatcher path (`pad mcp serve`) hands MCP input through to
      the CLI's --schema flag as inline JSON; previously fmt.Sprint(map)
      produced garbage Go-syntax. Generic — applies to any future flag
      that accepts JSON.

  3b. internal/mcp/dispatch_http_routes.go — mapCollectionCreate accepts
      `schema` (object or stringified JSON) alongside `fields`, errors
      if both are set, and backfills missing field labels using
      titleCaseLabel so HTTPHandlerDispatcher MCP clients see the same
      rendering as CLI users.

Tests cover: object→JSON encoding in BuildCLIArgs, array→JSON encoding,
structured schema input round-trips through mapCollectionCreate with
terminal_options preserved, missing-label backfill, stringified-JSON
fallback shape, both-flags rejection, malformed-JSON rejection.

Parent: PLAN-1333.

* fix(mcp): normalize empty/null schema input to absent per Codex round 1

Codex flagged a transport mismatch: the CLI treats `--schema ""` as
absent (falls through to --fields), but the HTTP dispatcher treated
schema=null or schema="" as set, triggering the mutually-exclusive
guard against fields or an invalid-JSON error from unmarshal.

Fix: in mapCollectionCreate, normalize nil and empty/whitespace strings
to "schema absent" before the mutex check. Keeps both transports
symmetric so MCP clients sending optional-arg defaults (null/empty) get
identical behavior to CLI users passing --schema "".

Test: TestMapCollectionCreate_EmptySchemaFallsThroughToFields covers
nil, "", and whitespace-only inputs in subtests, asserting the request
body uses the --fields-parsed schema.

Parent: PLAN-1333 / TASK-1335.
This commit is contained in:
xarmian
2026-05-10 21:35:55 -04:00
committed by GitHub
parent 25a21184a2
commit a28927c51e
6 changed files with 393 additions and 12 deletions
+10 -2
View File
@@ -83,10 +83,10 @@ type ToolSchema struct {
// ParamDef declares one parameter on a tool's input schema. Type maps
// to mcp-go's helper functions (WithString, WithNumber, WithBoolean,
// WithArray). Enum, when non-empty, constrains string parameters.
// WithArray, WithObject). Enum, when non-empty, constrains string parameters.
type ParamDef struct {
Name string
Type string // "string" | "number" | "bool" | "array<string>"
Type string // "string" | "number" | "bool" | "array<string>" | "object"
Description string
Enum []string
}
@@ -316,6 +316,14 @@ func paramDefToToolOption(p ParamDef) mcp.ToolOption {
return mcp.WithBoolean(p.Name, propOpts...)
case "array<string>":
return mcp.WithArray(p.Name, append(propOpts, mcp.WithStringItems())...)
case "object":
// Structured JSON parameter. The MCP host sees a generic object;
// per-tool semantics are encoded in the Description. Callers
// constructing the value can pass a native JSON object —
// BuildCLIArgs json-encodes it before handing it to the CLI as
// a string flag value (which the CLI's --schema flag accepts as
// inline JSON).
return mcp.WithObject(p.Name, propOpts...)
default:
return mcp.WithString(p.Name, propOpts...)
}
+14 -3
View File
@@ -27,7 +27,12 @@ var padCollectionTool = ToolDef{
{
Name: "fields",
Type: "string",
Description: "Field DSL: \"key:type[:options]; ...\". Optional for action=create. Example: \"status:select:open,done; priority:select:high,medium,low\".",
Description: "Compact field DSL: \"key:type[:options]; ...\". Optional for action=create. Example: \"status:select:open,done; priority:select:high,medium,low\". Use `schema` instead when you need terminal_options, custom defaults, computed fields, suffixes, or relation collections — the DSL cannot express those. Mutually exclusive with `schema`.",
},
{
Name: "schema",
Type: "object",
Description: "Structured CollectionSchema: {\"fields\":[{\"key\":\"...\",\"label\":\"...\",\"type\":\"...\",\"options\":[...],\"terminal_options\":[...],\"default\":\"...\",\"required\":bool,\"computed\":bool,\"suffix\":\"...\",\"collection\":\"...\"}]}. Optional for action=create. Use instead of `fields` when you need terminal_options or any FieldDef property the DSL cannot express. Missing `label` values are auto-filled from `key` using Title Case (e.g. due_date → Due Date). Mutually exclusive with `fields`.",
},
{
Name: "icon",
@@ -71,8 +76,14 @@ Actions:
Required: workspace.
create Create a new collection.
Required: workspace, name.
Optional: fields, icon, description, layout, default_view, board_group_by.
Field DSL example: "status:select:open,done; priority:select:high,medium,low".
Optional: fields OR schema (mutually exclusive), icon, description,
layout, default_view, board_group_by.
DSL example: fields="status:select:open,done; priority:select:high,medium,low"
Schema example: schema={"fields":[{"key":"status","type":"select","options":["new","done"],"terminal_options":["done"]}]}
Prefer schema when terminal_options matters (driving dashboard
active/complete counts) or when fields need defaults, computed
flags, suffixes, or relation collections the DSL cannot express
those.
Schema for an individual collection is included in the list response read it
from there rather than calling list again. v0.2 does not expose a dedicated
+34 -1
View File
@@ -446,7 +446,11 @@ func BuildCLIArgs(
}
continue
}
flagArgs = append(flagArgs, "--"+name, fmt.Sprint(val))
encoded, err := encodeFlagValue(val)
if err != nil {
return nil, fmt.Errorf("flag %q: %w", name, err)
}
flagArgs = append(flagArgs, "--"+name, encoded)
}
}
@@ -530,6 +534,35 @@ func toStringSlice(v any) ([]string, error) {
return nil, fmt.Errorf("expected array or string, got %T", v)
}
// encodeFlagValue stringifies an MCP input value for inclusion as a CLI
// flag argument. Strings and primitives pass through fmt.Sprint as before;
// structured values (maps, slices) are JSON-encoded so that flags accepting
// JSON literals — like `pad collection create --schema '<json>'` — receive
// a valid JSON string rather than Go's map-print syntax.
//
// exec.CommandContext takes []string args directly, so no shell-quoting is
// involved; the JSON string travels intact to the subprocess and is parsed
// by the CLI flag's own decoder.
func encodeFlagValue(v any) (string, error) {
switch t := v.(type) {
case nil:
return "", nil
case string:
return t, nil
case bool, int, int32, int64, uint, uint32, uint64, float32, float64, json.Number:
return fmt.Sprint(v), nil
default:
// map[string]any, []any, or any other structured type: JSON-encode.
// Compact JSON keeps the CLI process-arg list small and avoids any
// embedded-newline weirdness.
b, err := json.Marshal(v)
if err != nil {
return "", fmt.Errorf("encode value: %w", err)
}
return string(b), nil
}
}
func toBool(v any) (bool, error) {
switch t := v.(type) {
case bool:
+190
View File
@@ -383,6 +383,196 @@ func TestParseCollectionFieldsDSL_EmptyReturnsEmptyFields(t *testing.T) {
}
}
// TestMapCollectionCreate_AcceptsStructuredSchema verifies that the MCP
// dispatcher accepts a typed `schema` object input (the path TASK-1335
// adds for BUG-1284) and passes it through to the create-collection
// body with terminal_options preserved — which the DSL path cannot
// express.
func TestMapCollectionCreate_AcceptsStructuredSchema(t *testing.T) {
_, _, body, err := mapCollectionCreate(map[string]any{
"workspace": "docapp",
"name": "Marketing",
"schema": map[string]any{
"fields": []any{
map[string]any{
"key": "status",
"label": "Status",
"type": "select",
"options": []any{"idea", "drafting", "published", "archived"},
"terminal_options": []any{"published", "archived"},
"default": "idea",
"required": true,
},
},
},
})
if err != nil {
t.Fatalf("mapCollectionCreate: %v", err)
}
var payload map[string]any
if err := json.Unmarshal(body, &payload); err != nil {
t.Fatalf("decode body: %v", err)
}
schemaStr, _ := payload["schema"].(string)
var schema map[string]any
if err := json.Unmarshal([]byte(schemaStr), &schema); err != nil {
t.Fatalf("decode schema: %v", err)
}
fields, _ := schema["fields"].([]any)
if len(fields) != 1 {
t.Fatalf("fields length = %d, want 1", len(fields))
}
field, _ := fields[0].(map[string]any)
termOpts, _ := field["terminal_options"].([]any)
if len(termOpts) != 2 || termOpts[0] != "published" || termOpts[1] != "archived" {
t.Errorf("terminal_options not preserved through MCP body: %v", termOpts)
}
if field["default"] != "idea" {
t.Errorf("default not preserved: %v", field["default"])
}
if field["required"] != true {
t.Errorf("required not preserved: %v", field["required"])
}
}
// TestMapCollectionCreate_SchemaBackfillsMissingLabel mirrors the CLI's
// label-backfill heuristic — a schema field omitting `label` should get
// one auto-filled from `key` so the web UI doesn't render blank headers.
func TestMapCollectionCreate_SchemaBackfillsMissingLabel(t *testing.T) {
_, _, body, err := mapCollectionCreate(map[string]any{
"workspace": "docapp",
"name": "Marketing",
"schema": map[string]any{
"fields": []any{
map[string]any{
"key": "due_date",
"type": "date",
},
},
},
})
if err != nil {
t.Fatalf("mapCollectionCreate: %v", err)
}
var payload map[string]any
if err := json.Unmarshal(body, &payload); err != nil {
t.Fatalf("decode body: %v", err)
}
schemaStr, _ := payload["schema"].(string)
var schema map[string]any
if err := json.Unmarshal([]byte(schemaStr), &schema); err != nil {
t.Fatalf("decode schema: %v", err)
}
fields, _ := schema["fields"].([]any)
if len(fields) != 1 {
t.Fatalf("fields length = %d, want 1", len(fields))
}
field, _ := fields[0].(map[string]any)
if field["label"] != "Due Date" {
t.Errorf("label not backfilled: got %v, want \"Due Date\"", field["label"])
}
}
// TestMapCollectionCreate_SchemaAcceptsStringifiedJSON exercises the
// fallback shape for clients that can only pass strings — schema arrives
// as a JSON-encoded string and still round-trips through the body.
func TestMapCollectionCreate_SchemaAcceptsStringifiedJSON(t *testing.T) {
_, _, body, err := mapCollectionCreate(map[string]any{
"workspace": "docapp",
"name": "Marketing",
"schema": `{"fields":[{"key":"status","type":"select","options":["a","b"],"terminal_options":["b"]}]}`,
})
if err != nil {
t.Fatalf("mapCollectionCreate: %v", err)
}
var payload map[string]any
if err := json.Unmarshal(body, &payload); err != nil {
t.Fatalf("decode body: %v", err)
}
schemaStr, _ := payload["schema"].(string)
var schema map[string]any
if err := json.Unmarshal([]byte(schemaStr), &schema); err != nil {
t.Fatalf("decode schema: %v", err)
}
fields, _ := schema["fields"].([]any)
field, _ := fields[0].(map[string]any)
termOpts, _ := field["terminal_options"].([]any)
if len(termOpts) != 1 || termOpts[0] != "b" {
t.Errorf("terminal_options not preserved from stringified schema: %v", termOpts)
}
}
// TestMapCollectionCreate_FieldsAndSchemaMutuallyExclusive ensures the
// dispatcher rejects requests that set both flags — mirrors the CLI-side
// guard so MCP clients see the same error shape.
func TestMapCollectionCreate_FieldsAndSchemaMutuallyExclusive(t *testing.T) {
_, _, _, err := mapCollectionCreate(map[string]any{
"workspace": "docapp",
"name": "Both",
"fields": "status:select:open,done",
"schema": map[string]any{"fields": []any{}},
})
if err == nil {
t.Fatal("expected error when both fields and schema set, got nil")
}
if !strings.Contains(err.Error(), "mutually exclusive") {
t.Errorf("expected mutually-exclusive error, got: %v", err)
}
}
// TestMapCollectionCreate_EmptySchemaFallsThroughToFields verifies the
// transport-symmetry fix per Codex round 1: when a client sends
// schema=null or schema="" alongside fields, the dispatcher treats the
// empty schema as absent and uses the fields DSL — matching how the
// CLI handles an empty --schema value.
func TestMapCollectionCreate_EmptySchemaFallsThroughToFields(t *testing.T) {
cases := []struct {
name string
schema any
}{
{name: "nil-schema", schema: nil},
{name: "empty-string-schema", schema: ""},
{name: "whitespace-string-schema", schema: " "},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
_, _, body, err := mapCollectionCreate(map[string]any{
"workspace": "docapp",
"name": "Falls",
"fields": "status:select:open,done",
"schema": tc.schema,
})
if err != nil {
t.Fatalf("expected fall-through to --fields, got error: %v", err)
}
var payload map[string]any
if err := json.Unmarshal(body, &payload); err != nil {
t.Fatalf("decode body: %v", err)
}
schemaStr, _ := payload["schema"].(string)
if !strings.Contains(schemaStr, `"key":"status"`) {
t.Errorf("expected DSL-parsed status field, got schema: %q", schemaStr)
}
})
}
}
// TestMapCollectionCreate_SchemaMalformedJSON ensures a clear error when
// the stringified schema is invalid JSON.
func TestMapCollectionCreate_SchemaMalformedJSON(t *testing.T) {
_, _, _, err := mapCollectionCreate(map[string]any{
"workspace": "docapp",
"name": "Bad",
"schema": `{not valid json`,
})
if err == nil {
t.Fatal("expected error for malformed schema JSON, got nil")
}
if !strings.Contains(err.Error(), "invalid schema JSON") {
t.Errorf("expected 'invalid schema JSON' in error, got: %v", err)
}
}
// --- library list ---
func TestDispatch_LibraryList_BothEndpoints(t *testing.T) {
+78 -6
View File
@@ -9,6 +9,7 @@ import (
"strings"
"github.com/PerpetualSoftware/pad/internal/collections"
"github.com/PerpetualSoftware/pad/internal/models"
)
// routeSpec is the declarative description of a CLI→HTTP mapping.
@@ -361,13 +362,46 @@ func mapCollectionCreate(input map[string]any) (string, string, []byte, error) {
}
dsl, _ := input["fields"].(string)
schema, err := parseCollectionFieldsDSL(dsl)
if err != nil {
return "", "", nil, fmt.Errorf("parse --fields: %w", err)
rawSchema, hasSchema := input["schema"]
// Normalize "schema present but empty" — null and empty string — to
// absent. Without this, MCP clients sending `schema: null` (or `""`)
// while also setting `fields` would hit the mutually-exclusive guard,
// even though the CLI side treats an empty --schema value as a
// fall-through to --fields. Keeps the two transports symmetric.
if hasSchema {
switch v := rawSchema.(type) {
case nil:
hasSchema = false
case string:
if strings.TrimSpace(v) == "" {
hasSchema = false
}
}
}
schemaJSON, err := json.Marshal(schema)
if err != nil {
return "", "", nil, fmt.Errorf("encode schema: %w", err)
if hasSchema && dsl != "" {
return "", "", nil, fmt.Errorf("fields and schema are mutually exclusive")
}
var schemaJSON []byte
if hasSchema {
// Schema may arrive as a map (typed object param), as a string
// (agent passed a stringified JSON), or — defensively — as nil
// when omitted but the key was still set.
encoded, err := encodeSchemaForBody(rawSchema)
if err != nil {
return "", "", nil, err
}
schemaJSON = encoded
} else {
schema, err := parseCollectionFieldsDSL(dsl)
if err != nil {
return "", "", nil, fmt.Errorf("parse --fields: %w", err)
}
b, err := json.Marshal(schema)
if err != nil {
return "", "", nil, fmt.Errorf("encode schema: %w", err)
}
schemaJSON = b
}
layout, _ := input["layout"].(string)
@@ -470,6 +504,44 @@ func parseCollectionFieldsDSL(dsl string) (map[string]any, error) {
return out, nil
}
// encodeSchemaForBody converts a `schema` MCP input value into a
// JSON-encoded CollectionSchema string suitable for the create-collection
// HTTP body's `schema` field. Accepts either a structured object (the
// typical typed-param shape an MCP client sends) or a string containing
// inline JSON (fallback for clients that can't construct typed objects).
//
// Backfills missing `label` values on each field using the same
// Title-Case-of-key heuristic as the CLI (`cmd/pad/main.go`'s
// collectionSchemaJSONFromFlags) so a schema constructed in either
// surface renders identically in the web UI.
func encodeSchemaForBody(raw any) ([]byte, error) {
if raw == nil {
return json.Marshal(models.CollectionSchema{})
}
var blob []byte
switch t := raw.(type) {
case string:
blob = []byte(t)
default:
b, err := json.Marshal(raw)
if err != nil {
return nil, fmt.Errorf("encode schema: %w", err)
}
blob = b
}
var schema models.CollectionSchema
if err := json.Unmarshal(blob, &schema); err != nil {
return nil, fmt.Errorf("invalid schema JSON: %w", err)
}
for i := range schema.Fields {
if schema.Fields[i].Label == "" && schema.Fields[i].Key != "" {
schema.Fields[i].Label = titleCaseLabel(schema.Fields[i].Key)
}
}
return json.Marshal(schema)
}
// titleCaseLabel converts a snake_case key into a Title Case label
// the same way the CLI does ("due_date" → "Due Date"). Avoids
// pulling in golang.org/x/text/cases for a one-line transformation.
+67
View File
@@ -411,3 +411,70 @@ func TestExecDispatcher_NonzeroExitReturnsErrorResult(t *testing.T) {
t.Errorf("expected IsError result for non-zero exit")
}
}
// TestBuildCLIArgs_ObjectFlagValueIsJSONEncoded covers TASK-1335: when an
// MCP client passes a structured object as a flag value (e.g. `schema`
// for `pad collection create`), BuildCLIArgs must JSON-encode it so the
// CLI receives a parseable string rather than Go's default `map[...]` print.
func TestBuildCLIArgs_ObjectFlagValueIsJSONEncoded(t *testing.T) {
cmd := cmdhelp.Command{
Flags: map[string]cmdhelp.Flag{
"schema": {Type: "string"},
},
}
got, err := BuildCLIArgs(cmd, map[string]any{
"schema": map[string]any{
"fields": []any{
map[string]any{"key": "status", "type": "select"},
},
},
}, "", nil)
if err != nil {
t.Fatalf("BuildCLIArgs: %v", err)
}
// Find the --schema value.
var schemaVal string
for i := 0; i < len(got)-1; i++ {
if got[i] == "--schema" {
schemaVal = got[i+1]
break
}
}
if schemaVal == "" {
t.Fatalf("--schema flag not emitted; got %v", got)
}
// Must be valid JSON.
if !strings.HasPrefix(schemaVal, "{") {
t.Errorf("expected JSON-encoded value, got %q", schemaVal)
}
if !strings.Contains(schemaVal, `"status"`) || !strings.Contains(schemaVal, `"select"`) {
t.Errorf("encoded value does not preserve field shape: %q", schemaVal)
}
}
// TestBuildCLIArgs_ArrayFlagValueIsJSONEncoded covers the array branch
// of the same object-encoding path — confirms a top-level array (not just
// objects) gets JSON-encoded too.
func TestBuildCLIArgs_ArrayFlagValueIsJSONEncoded(t *testing.T) {
cmd := cmdhelp.Command{
Flags: map[string]cmdhelp.Flag{
"raw": {Type: "string"},
},
}
got, err := BuildCLIArgs(cmd, map[string]any{
"raw": []any{"a", "b", "c"},
}, "", nil)
if err != nil {
t.Fatalf("BuildCLIArgs: %v", err)
}
var rawVal string
for i := 0; i < len(got)-1; i++ {
if got[i] == "--raw" {
rawVal = got[i+1]
break
}
}
if rawVal != `["a","b","c"]` {
t.Errorf("expected JSON array, got %q", rawVal)
}
}