Skip to content

fix(native client): [sc-145675] Correct MCP tool contracts and parameter descriptions #168

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
noway merged 1 commit into from
May 29, 2026
Merged

fix(native client): [sc-145675] Correct MCP tool contracts and parameter descriptions #168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

- Aligned CLI flag types, values and validation with what the public API actually accepts: `codes --max-redemptions 0` / `--expire-after 0` now send `null` (unlimited / clear) rather than 422; `maps paths --cost` is an integer and `--cost-override` a boolean; `maps paths --direction` accepts `bidirectional | forward | backward`; `maps layers features create` requires `--name`; `collections items --radius` is guarded `>= 1`; `screens sections prices`/`hours` honour `--lang` for their translated fields; `CrossRegionLink` is rejected client-side for collection-item and link-item types; media types validate through one shared `api.ValidateMediaType`; and `codes list --sort-field` no longer advertises the unsupported `redemptions`.
- Corrected MCP tool contracts: `list_collection_items` returns its rows under a `collection_items` key (was a generic `items`); `create_collection`/`screen`/`section`/`code`/`media`/`map_feature` document their server-required fields; pagination params are described consistently (server default 30, max 1000) instead of four contradictory phrasings; `delete_media` gained a per-locale `language` arg; and `update_collection`/`update_screen` note the `primary_language`-flip caveat.

## [0.10.33] - 2026-05-28

Expand Down
70 changes: 64 additions & 6 deletions internal/mcp/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ func TestResolveClientNoConfigError(t *testing.T) {
orig, _ := os.Getwd()
defer os.Chdir(orig)
os.Chdir(dir)

t.Setenv("STQRY_CONFIG_HOME", dir)

_, err := stqrymcp.ResolveClient(nil)
Expand Down Expand Up @@ -286,6 +286,46 @@ func TestResolveClientSessionPriorityOverDisk(t *testing.T) {
}
}

// ---- list_collection_items ----

// TestListCollectionItemsKey locks in the wrapper-key fix: list_collection_items
// must return its rows under "collection_items" (the server's response key and
// the convention every other list tool follows), not a generic "items".
func TestListCollectionItemsKey(t *testing.T) {
mock := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/public/collections/42/collection_items" {
http.NotFound(w, r)
return
}
w.Header().Set("Content-Type", "application/json")
fmt.Fprint(w, `{"collection_items":[{"id":"7","item_type":"Screen","item_id":"99"}],"meta":{"page":1,"pages":1,"per_page":30,"count":1}}`)
}))
defer mock.Close()

dir := t.TempDir()
cfg := &config.DirectoryConfig{APIKey: "tok", APIURL: mock.URL}
if err := config.SaveDirectoryConfig(dir, cfg); err != nil {
t.Fatal(err)
}
orig, _ := os.Getwd()
defer os.Chdir(orig)
os.Chdir(dir)

s := stqrymcp.NewServer()
result := callTool(s, "list_collection_items", `{"collection_id":"42"}`)
if result == nil || result.IsError {
t.Fatalf("expected success, got: %v", result)
}
txt := toolText(result)
if !strings.Contains(txt, `"collection_items"`) {
t.Errorf("expected rows under \"collection_items\" key, got: %s", txt)
}
// The lone bare-"items" key was the bug; make sure it didn't come back.
if strings.Contains(txt, `"items"`) {
t.Errorf("did not expect a bare \"items\" key, got: %s", txt)
}
}

// ---- list_projects ----

func TestListProjectsHappyPath(t *testing.T) {
Expand Down Expand Up @@ -637,8 +677,8 @@ func TestCreateMediaInvalidType(t *testing.T) {
if !result.IsError {
t.Fatal("expected error for invalid type")
}
if !strings.Contains(toolText(result), "invalid type") {
t.Errorf("expected helpful error mentioning invalid type, got: %s", toolText(result))
if !strings.Contains(toolText(result), "invalid media type") {
t.Errorf("expected helpful error mentioning invalid media type, got: %s", toolText(result))
}
}

Expand All @@ -652,6 +692,25 @@ func TestCreateMediaMissingFilePath(t *testing.T) {
}
}

func TestCreateMediaMissingName(t *testing.T) {
// MediaItem requires a name for every subtype and the public API derives
// none from the filename, so create_media must reject a blank name
// client-side (before uploading) rather than orphan a file and 422.
dir := t.TempDir()
filePath := filepath.Join(dir, "f.mp4")
if err := os.WriteFile(filePath, []byte("x"), 0600); err != nil {
t.Fatal(err)
}
s := stqrymcp.NewServer()
result := callTool(s, "create_media", fmt.Sprintf(`{"file_path":%q,"type":"video"}`, filePath))
if result == nil || !result.IsError {
t.Fatal("expected error for missing name")
}
if !strings.Contains(toolText(result), "name is required") {
t.Errorf("expected 'name is required' error, got: %s", toolText(result))
}
}

func TestCreateMediaMissingType(t *testing.T) {
dir := t.TempDir()
filePath := filepath.Join(dir, "f.mp4")
Expand All @@ -678,7 +737,7 @@ func TestCreateMediaBadFilePath(t *testing.T) {
os.Chdir(dir)

s := stqrymcp.NewServer()
result := callTool(s, "create_media", `{"file_path":"/nonexistent/file.mp4","type":"video"}`)
result := callTool(s, "create_media", `{"file_path":"/nonexistent/file.mp4","type":"video","name":"Test"}`)
if result == nil || !result.IsError {
t.Fatal("expected error for non-existent file path")
}
Expand Down Expand Up @@ -796,9 +855,8 @@ func TestCreateMediaUploadError(t *testing.T) {
os.Chdir(dir)

s := stqrymcp.NewServer()
result := callTool(s, "create_media", fmt.Sprintf(`{"file_path":%q,"type":"video"}`, filePath))
result := callTool(s, "create_media", fmt.Sprintf(`{"file_path":%q,"type":"video","name":"Test"}`, filePath))
if result == nil || !result.IsError {
t.Fatal("expected error when upload API returns 500")
}
}

12 changes: 4 additions & 8 deletions internal/mcp/tools_codes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,8 @@ func registerCodeTools(s *server.MCPServer, sess *Session) {
s.AddTool(
mcpgo.NewTool("list_codes",
mcpgo.WithDescription("List all codes for the configured STQRY account."),
mcpgo.WithNumber("page",
mcpgo.Description("Page number (1-based, default: 1)"),
),
mcpgo.WithNumber("per_page",
mcpgo.Description("Items per page (default: 25)"),
),
pageParam(),
perPageParam(),
),
func(ctx context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
client, err := ResolveClient(sess)
Expand Down Expand Up @@ -66,10 +62,10 @@ func registerCodeTools(s *server.MCPServer, sess *Session) {
// create_code: creates a new code
s.AddTool(
mcpgo.NewTool("create_code",
mcpgo.WithDescription("Create a new STQRY code."),
mcpgo.WithDescription("Create a new STQRY code. Required: coupon_code, linked_type, linked_id, project_id. valid_from and valid_to must be set together (setting one without the other is rejected)."),
mcpgo.WithObject("fields",
mcpgo.Required(),
mcpgo.Description("Arbitrary JSON object of code fields to set"),
mcpgo.Description("Code fields. Required: coupon_code, linked_type (the linked resource's type, e.g. Collection), linked_id, project_id. Optional: valid_from + valid_to (set both or neither), max_redemptions (integer >= 1; OMIT or send null for unlimited — a literal 0 is rejected), expire_after, timezone, tags. Example: {\"coupon_code\": \"SUMMER\", \"linked_type\": \"Collection\", \"linked_id\": 42, \"project_id\": 7}."),
),
),
func(ctx context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
Expand Down
26 changes: 9 additions & 17 deletions internal/mcp/tools_collections.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,8 @@ func registerCollectionTools(s *server.MCPServer, sess *Session) {
s.AddTool(
mcpgo.NewTool("list_collections",
mcpgo.WithDescription("List all collections for the configured STQRY account."),
mcpgo.WithNumber("page",
mcpgo.Description("Page number (1-based, default: 1)"),
),
mcpgo.WithNumber("per_page",
mcpgo.Description("Items per page (default: 25)"),
),
pageParam(),
perPageParam(),
),
func(ctx context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
client, err := ResolveClient(sess)
Expand Down Expand Up @@ -66,10 +62,10 @@ func registerCollectionTools(s *server.MCPServer, sess *Session) {
// create_collection: creates a new collection with the given fields
s.AddTool(
mcpgo.NewTool("create_collection",
mcpgo.WithDescription("Create a new STQRY collection."),
mcpgo.WithDescription("Create a new STQRY collection. Required: type (one of: list, tour, organization, menu, search), name, and primary_language (e.g. \"en\"). The server also requires title and short_title on the primary language; pass translated fields as locale maps, e.g. {\"en\": \"…\"}."),
mcpgo.WithObject("fields",
mcpgo.Required(),
mcpgo.Description("Arbitrary JSON object of collection fields to set"),
mcpgo.Description("Collection fields. Required: type, name, primary_language, plus title and short_title on the primary language. Example: {\"type\": \"tour\", \"name\": \"City Walk\", \"primary_language\": \"en\", \"title\": {\"en\": \"City Walk\"}, \"short_title\": {\"en\": \"Walk\"}}. Optional: description, tour_type, the *_media_item_id covers/logo/preview, map_view_enabled."),
),
),
func(ctx context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
Expand All @@ -93,7 +89,7 @@ func registerCollectionTools(s *server.MCPServer, sess *Session) {
// update_collection: updates an existing collection by ID
s.AddTool(
mcpgo.NewTool("update_collection",
mcpgo.WithDescription("Update an existing STQRY collection by ID."),
mcpgo.WithDescription("Update an existing STQRY collection by ID. Translated fields (title, short_title, description) are locale maps, e.g. {\"en\": \"…\"}. Pass primary_language to flip the primary locale — the target locale must already have title + short_title populated, or the server rejects the flip."),
mcpgo.WithString("id",
mcpgo.Required(),
mcpgo.Description("The collection ID"),
Expand Down Expand Up @@ -165,12 +161,8 @@ func registerCollectionTools(s *server.MCPServer, sess *Session) {
mcpgo.Required(),
mcpgo.Description("The collection ID"),
),
mcpgo.WithNumber("page",
mcpgo.Description("Page number (1-based, default: 1)"),
),
mcpgo.WithNumber("per_page",
mcpgo.Description("Items per page (default: 25)"),
),
pageParam(),
perPageParam(),
),
func(ctx context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
collectionID := req.GetString("collection_id", "")
Expand All @@ -186,8 +178,8 @@ func registerCollectionTools(s *server.MCPServer, sess *Session) {
return mcpgo.NewToolResultError(fmt.Sprintf("listing collection items: %v", err)), nil
}
return jsonResult(map[string]interface{}{
"items": items,
"meta": meta,
"collection_items": items,
"meta": meta,
})
},
)
Expand Down
68 changes: 46 additions & 22 deletions internal/mcp/tools_maps.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ func registerMapCRUD(s *server.MCPServer, sess *Session) {
s.AddTool(
mcpgo.NewTool("list_maps",
mcpgo.WithDescription("List all maps in the configured STQRY account."),
mcpgo.WithNumber("page", mcpgo.Description("Page number (default 1)")),
mcpgo.WithNumber("per_page", mcpgo.Description("Items per page (default 25)")),
pageParam(),
perPageParam(),
mcpgo.WithString("q", mcpgo.Description("Search query (matches map name)")),
),
func(ctx context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
Expand Down Expand Up @@ -200,9 +200,15 @@ func registerMapNestedCRUD(s *server.MCPServer, sess *Session) {
}
return map[string]interface{}{"map_layers": items, "meta": meta}, nil
},
get: func(c *api.Client, mapID, id string) (map[string]interface{}, error) { return api.GetMapLayer(c, mapID, id) },
create: func(c *api.Client, mapID string, fields map[string]interface{}) (map[string]interface{}, error) { return api.CreateMapLayer(c, mapID, fields) },
update: func(c *api.Client, mapID, id string, fields map[string]interface{}) (map[string]interface{}, error) { return api.UpdateMapLayer(c, mapID, id, fields) },
get: func(c *api.Client, mapID, id string) (map[string]interface{}, error) {
return api.GetMapLayer(c, mapID, id)
},
create: func(c *api.Client, mapID string, fields map[string]interface{}) (map[string]interface{}, error) {
return api.CreateMapLayer(c, mapID, fields)
},
update: func(c *api.Client, mapID, id string, fields map[string]interface{}) (map[string]interface{}, error) {
return api.UpdateMapLayer(c, mapID, id, fields)
},
delete: func(c *api.Client, mapID, id string) error { return api.DeleteMapLayer(c, mapID, id) },
})

Expand Down Expand Up @@ -247,9 +253,15 @@ func registerMapNestedCRUD(s *server.MCPServer, sess *Session) {
}
return map[string]interface{}{"map_pois": items, "meta": meta}, nil
},
get: func(c *api.Client, mapID, id string) (map[string]interface{}, error) { return api.GetMapPoi(c, mapID, id) },
create: func(c *api.Client, mapID string, fields map[string]interface{}) (map[string]interface{}, error) { return api.CreateMapPoi(c, mapID, fields) },
update: func(c *api.Client, mapID, id string, fields map[string]interface{}) (map[string]interface{}, error) { return api.UpdateMapPoi(c, mapID, id, fields) },
get: func(c *api.Client, mapID, id string) (map[string]interface{}, error) {
return api.GetMapPoi(c, mapID, id)
},
create: func(c *api.Client, mapID string, fields map[string]interface{}) (map[string]interface{}, error) {
return api.CreateMapPoi(c, mapID, fields)
},
update: func(c *api.Client, mapID, id string, fields map[string]interface{}) (map[string]interface{}, error) {
return api.UpdateMapPoi(c, mapID, id, fields)
},
delete: func(c *api.Client, mapID, id string) error { return api.DeleteMapPoi(c, mapID, id) },
})

Expand All @@ -264,9 +276,15 @@ func registerMapNestedCRUD(s *server.MCPServer, sess *Session) {
}
return map[string]interface{}{"map_path_nodes": items, "meta": meta}, nil
},
get: func(c *api.Client, mapID, id string) (map[string]interface{}, error) { return api.GetMapPathNode(c, mapID, id) },
create: func(c *api.Client, mapID string, fields map[string]interface{}) (map[string]interface{}, error) { return api.CreateMapPathNode(c, mapID, fields) },
update: func(c *api.Client, mapID, id string, fields map[string]interface{}) (map[string]interface{}, error) { return api.UpdateMapPathNode(c, mapID, id, fields) },
get: func(c *api.Client, mapID, id string) (map[string]interface{}, error) {
return api.GetMapPathNode(c, mapID, id)
},
create: func(c *api.Client, mapID string, fields map[string]interface{}) (map[string]interface{}, error) {
return api.CreateMapPathNode(c, mapID, fields)
},
update: func(c *api.Client, mapID, id string, fields map[string]interface{}) (map[string]interface{}, error) {
return api.UpdateMapPathNode(c, mapID, id, fields)
},
delete: func(c *api.Client, mapID, id string) error { return api.DeleteMapPathNode(c, mapID, id) },
})

Expand All @@ -281,9 +299,15 @@ func registerMapNestedCRUD(s *server.MCPServer, sess *Session) {
}
return map[string]interface{}{"map_paths": items, "meta": meta}, nil
},
get: func(c *api.Client, mapID, id string) (map[string]interface{}, error) { return api.GetMapPath(c, mapID, id) },
create: func(c *api.Client, mapID string, fields map[string]interface{}) (map[string]interface{}, error) { return api.CreateMapPath(c, mapID, fields) },
update: func(c *api.Client, mapID, id string, fields map[string]interface{}) (map[string]interface{}, error) { return api.UpdateMapPath(c, mapID, id, fields) },
get: func(c *api.Client, mapID, id string) (map[string]interface{}, error) {
return api.GetMapPath(c, mapID, id)
},
create: func(c *api.Client, mapID string, fields map[string]interface{}) (map[string]interface{}, error) {
return api.CreateMapPath(c, mapID, fields)
},
update: func(c *api.Client, mapID, id string, fields map[string]interface{}) (map[string]interface{}, error) {
return api.UpdateMapPath(c, mapID, id, fields)
},
delete: func(c *api.Client, mapID, id string) error { return api.DeleteMapPath(c, mapID, id) },
})

Expand Down Expand Up @@ -319,8 +343,8 @@ func registerMapNestedCRUD(s *server.MCPServer, sess *Session) {
mcpgo.WithDescription("List GeoJSON features in a map layer."),
mapIDArg,
mcpgo.WithString("layer_id", mcpgo.Required(), mcpgo.Description("Layer ID")),
mcpgo.WithNumber("page", mcpgo.Description("Page number")),
mcpgo.WithNumber("per_page", mcpgo.Description("Items per page")),
pageParam(),
perPageParam(),
),
func(ctx context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
mapID := req.GetString("map_id", "")
Expand Down Expand Up @@ -369,10 +393,10 @@ func registerMapNestedCRUD(s *server.MCPServer, sess *Session) {

s.AddTool(
mcpgo.NewTool("create_map_feature",
mcpgo.WithDescription("Create a GeoJSON feature in a map layer. `fields` must include geojson; may include name, label, settings, map_poi_id."),
mcpgo.WithDescription("Create a GeoJSON feature in a map layer. `fields` must include geojson and name (both required server-side); may include label, settings, map_poi_id."),
mapIDArg,
mcpgo.WithString("layer_id", mcpgo.Required(), mcpgo.Description("Layer ID")),
mcpgo.WithObject("fields", mcpgo.Required(), mcpgo.Description("Feature fields including geojson")),
mcpgo.WithObject("fields", mcpgo.Required(), mcpgo.Description("Feature fields; must include geojson and name")),
),
func(ctx context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
mapID := req.GetString("map_id", "")
Expand Down Expand Up @@ -474,8 +498,8 @@ func addNestedCRUD(s *server.MCPServer, sess *Session, r nestedMapResource) {
mcpgo.NewTool(r.listTool,
mcpgo.WithDescription(fmt.Sprintf("List %ss for a map.", r.humanName)),
r.parentArg,
mcpgo.WithNumber("page", mcpgo.Description("Page number")),
mcpgo.WithNumber("per_page", mcpgo.Description("Items per page")),
pageParam(),
perPageParam(),
),
func(ctx context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
mapID := req.GetString("map_id", "")
Expand Down Expand Up @@ -694,8 +718,8 @@ func addLinkableCRUD(s *server.MCPServer, sess *Session, r linkableMapResource)
s.AddTool(
mcpgo.NewTool(r.listTool,
mcpgo.WithDescription(fmt.Sprintf("List %ss. Pass at most one of collection_id or story_section_id to scope.", r.humanName)),
mcpgo.WithNumber("page", mcpgo.Description("Page number")),
mcpgo.WithNumber("per_page", mcpgo.Description("Items per page")),
pageParam(),
perPageParam(),
mcpgo.WithString("collection_id", mcpgo.Description("Filter by parent collection")),
mcpgo.WithString("story_section_id", mcpgo.Description("Filter by parent story section")),
),
Expand Down
Loading
Loading