[object Object]

← back to Cli Printing Press

fix(cli): tools-manifest.json classifies reclassified path params as path, not query (#394)

cdd9153b15e9c3a746c612fe12195ce3da9f68f1 · 2026-04-29 12:07:19 -0700 · Trevin Chow

reclassifyPathParamModifiers (parser.go) converts enum-typed,
date-named, paginated, or default-bearing path params from positional
args into CLI flags (so users type `disconnect --calendar=google`
instead of `disconnect google`). It sets Positional=false but keeps
PathParam=true on the spec.Param to preserve the URL-substitution
contract.

buildManifestTool was only inspecting Positional when classifying
location, so reclassified path params landed in tools-manifest.json
as `location: "query"`. Three downstream consumers misread that as
"this isn't in the URL":

- The mcp-descriptions.json override agent: described
  /v2/calendars/{calendar}/disconnect's `calendar` param as
  "Optional: calendar" while the spec marks it required path. Cal-com
  shipped 19 tools with inverted required/optional claims for path
  params on calendar-scoped endpoints because the manifest told the
  agent they were query params.
- Audit / scorer: reads location to decide whether a param is part
  of the URL contract or just a filter.
- MCP hosts: a reasonable host could refuse path-substituted params
  appearing in a "query" slot.

Fix: classify location as "path" when Positional || PathParam. The
test pins the contract for an enum-typed reclassified path param
(matches cal-com's /v2/calendars/{calendar} shape) and is the kind
of regression guard that would have caught this bug at write time.

Files touched

Diff

commit cdd9153b15e9c3a746c612fe12195ce3da9f68f1
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Wed Apr 29 12:07:19 2026 -0700

    fix(cli): tools-manifest.json classifies reclassified path params as path, not query (#394)
    
    reclassifyPathParamModifiers (parser.go) converts enum-typed,
    date-named, paginated, or default-bearing path params from positional
    args into CLI flags (so users type `disconnect --calendar=google`
    instead of `disconnect google`). It sets Positional=false but keeps
    PathParam=true on the spec.Param to preserve the URL-substitution
    contract.
    
    buildManifestTool was only inspecting Positional when classifying
    location, so reclassified path params landed in tools-manifest.json
    as `location: "query"`. Three downstream consumers misread that as
    "this isn't in the URL":
    
    - The mcp-descriptions.json override agent: described
      /v2/calendars/{calendar}/disconnect's `calendar` param as
      "Optional: calendar" while the spec marks it required path. Cal-com
      shipped 19 tools with inverted required/optional claims for path
      params on calendar-scoped endpoints because the manifest told the
      agent they were query params.
    - Audit / scorer: reads location to decide whether a param is part
      of the URL contract or just a filter.
    - MCP hosts: a reasonable host could refuse path-substituted params
      appearing in a "query" slot.
    
    Fix: classify location as "path" when Positional || PathParam. The
    test pins the contract for an enum-typed reclassified path param
    (matches cal-com's /v2/calendars/{calendar} shape) and is the kind
    of regression guard that would have caught this bug at write time.
---
 internal/pipeline/toolsmanifest.go      | 12 ++++++--
 internal/pipeline/toolsmanifest_test.go | 52 +++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/internal/pipeline/toolsmanifest.go b/internal/pipeline/toolsmanifest.go
index 1040b2f9..4c052b00 100644
--- a/internal/pipeline/toolsmanifest.go
+++ b/internal/pipeline/toolsmanifest.go
@@ -198,10 +198,18 @@ func buildManifestTool(name, description string, ep spec.Endpoint) ManifestTool
 		Params:      make([]ManifestParam, 0, len(ep.Params)+len(ep.Body)),
 	}
 
-	// Regular params: positional → path, others → query.
+	// Regular params. A param ends up at "path" when the runtime
+	// substitutes it into the URL — that's true for both positional
+	// path args (Positional=true) AND path params reclassified into
+	// CLI flags by reclassifyPathParamModifiers (PathParam=true, e.g.,
+	// enum-typed path params like /v2/calendars/{calendar} that the CLI
+	// renders as --calendar). Without this OR, reclassified path params
+	// land in the manifest as location: "query", which misleads
+	// description-override agents reading the manifest to understand
+	// the API contract.
 	for _, p := range ep.Params {
 		loc := "query"
-		if p.Positional {
+		if p.Positional || p.PathParam {
 			loc = "path"
 		}
 		tool.Params = append(tool.Params, ManifestParam{
diff --git a/internal/pipeline/toolsmanifest_test.go b/internal/pipeline/toolsmanifest_test.go
index 7b226509..5cf7f15b 100644
--- a/internal/pipeline/toolsmanifest_test.go
+++ b/internal/pipeline/toolsmanifest_test.go
@@ -234,6 +234,58 @@ func TestWriteToolsManifest_ParamLocationClassification(t *testing.T) {
 	assert.False(t, tool.Params[3].Required)
 }
 
+// TestWriteToolsManifest_ReclassifiedPathParamKeepsPathLocation pins
+// the path location for path params that reclassifyPathParamModifiers
+// converted from positional args to flags (e.g., enum-typed path
+// params like /v2/calendars/{calendar} where calendar has
+// enum: [apple, google, office365]). Without checking PathParam
+// alongside Positional, these end up location: "query" and the
+// description-override agent can't tell they're URL-substituted.
+func TestWriteToolsManifest_ReclassifiedPathParamKeepsPathLocation(t *testing.T) {
+	dir := t.TempDir()
+	parsed := &spec.APISpec{
+		Name:    "test-api",
+		BaseURL: "https://api.test.com",
+		Auth:    spec.AuthConfig{Type: "none"},
+		Resources: map[string]spec.Resource{
+			"Calendars": {
+				Endpoints: map[string]spec.Endpoint{
+					"Disconnect": {
+						Method: "POST",
+						Path:   "/calendars/{calendar}/disconnect",
+						Params: []spec.Param{
+							{
+								Name:        "calendar",
+								Type:        "string",
+								Description: "Calendar provider",
+								Enum:        []string{"apple", "google", "office365"},
+								Default:     "apple",
+								Positional:  false, // reclassified: enum + default → flag
+								PathParam:   true,  // ...but still substituted into URL
+								Required:    false, // CLI default fills in if omitted
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	require.NoError(t, WriteToolsManifest(dir, parsed))
+
+	data, err := os.ReadFile(filepath.Join(dir, ToolsManifestFilename))
+	require.NoError(t, err)
+	var got ToolsManifest
+	require.NoError(t, json.Unmarshal(data, &got))
+
+	require.Len(t, got.Tools, 1)
+	require.Len(t, got.Tools[0].Params, 1)
+	calendar := got.Tools[0].Params[0]
+	assert.Equal(t, "calendar", calendar.Name)
+	assert.Equal(t, "path", calendar.Location, "reclassified path param must still be location: path")
+	assert.False(t, calendar.Required, "default fills in if omitted; agent may skip the param")
+}
+
 func TestWriteToolsManifest_AuthConfigRoundTrip(t *testing.T) {
 	dir := t.TempDir()
 	parsed := &spec.APISpec{

← 64f67981 feat(cli): mcp-sync reads display_name from public library r  ·  back to Cli Printing Press  ·  docs(skills): polish skill — read spec.json directly for req e326adfd →