diff --git a/docs/docs/reference/mcp-tools.md b/docs/docs/reference/mcp-tools.md index 47e8a2d..7f532c6 100644 --- a/docs/docs/reference/mcp-tools.md +++ b/docs/docs/reference/mcp-tools.md @@ -61,9 +61,11 @@ The structured content itself is the array described above, not an object that w | Field | Meaning | | --- | --- | | `name` | Exact enabled dotted capability name, such as `records.lookup`. | -| `signature` | Keyword-only Starlark signature generated from the registered Go binding. | +| `signature` | Invocation-only keyword signature. It ends after the parameter list and never contains a Go output type. | | `summary` | Registered compact summary. | +`signature` contains the dotted name, a `*` keyword-only marker when there are parameters, and the ordered input fields with their type notations. It ends at `)`. The exact forms are `records.lookup(*, key: str, limit: int | None)` and `records.status()`. The result contract is `describe_api.output`, not `signature`. + ## `describe_api` Describe one enabled capability by the exact name returned by search_api, without whitespace or case changes. @@ -143,6 +145,10 @@ For the site-wide sample, the requested name is `records.lookup`. Its stable ID, The sample capability therefore describes input fields `key` (`str`, required) and `limit` (`int | None`, optional), and output fields `key` (`str`, required) and `count` (`int`, required). +`signature` is the same invocation-only keyword signature returned by `search_api`. + +`output` is the stable result contract. Models and clients must use these field shapes. Clients that parsed a trailing ` -> ` suffix on `signature` must stop parsing it and use `describe_api.output` instead. There is no compatibility suffix, alias, or alternate signature field. + ## `execute` Execute one Starlark program that defines def main(): with zero arguments, calls only names confirmed through search_api and describe_api inside main, and returns main's final result. @@ -202,6 +208,7 @@ The listed descriptions above are the model-facing contract. Recovery uses the s - Search with a short literal substring over enabled names and summaries. If the result is empty, retry with a shorter term. - Pass `describe_api` the exact `name` returned by `search_api`, without whitespace or case changes. - Compare native call arguments with the `describe_api` field shapes before `execute`. +- Use `describe_api.output` as the result contract. Do not parse a type name from `signature`. - Define `def main():` with zero arguments. Call only names confirmed through `search_api` and `describe_api` inside `main`. Return `main`'s final result. - After `resource limit exceeded`, reduce program or result complexity and retry. - `permission denied` and `authorization policy failure` are outcomes for that call only. They do not disclose policy rules. diff --git a/docs/docs/reference/public-api.md b/docs/docs/reference/public-api.md index 5569fab..7442da1 100644 --- a/docs/docs/reference/public-api.md +++ b/docs/docs/reference/public-api.md @@ -141,9 +141,11 @@ Search(query string) ([]SearchResult, error) | Field | Type | Meaning | | --- | --- | --- | | `name` | string | Exact enabled dotted name. | -| `signature` | string | Keyword-only Starlark signature generated from the registered binding. | +| `signature` | string | Invocation-only keyword signature. It ends after the parameter list and never contains a Go output type. | | `summary` | string | Registered summary. | +`signature` contains the dotted name, a `*` keyword-only marker when there are parameters, and the ordered input fields with their type notations. It ends at `)`. The exact forms are `records.lookup(*, key: str, limit: int | None)` and `records.status()`. The result contract is `Description.Output`, not `signature`. + An oversized query returns `ErrResourceLimit`. An unexpected server-state failure returns `ErrInternal`. #### `Describe` @@ -159,14 +161,16 @@ Describe(name CapabilityName) (Description, error) | JSON field | Type | Meaning | | --- | --- | --- | | `name` | string | Exact dotted name. | -| `signature` | string | Generated Starlark signature. | +| `signature` | string | Invocation-only keyword signature. It ends after `)` and never contains a Go output type. | | `summary` | string | Registered summary. | | `description` | string | Registered full description. | | `input` | array of field shapes | Ordered input fields. | -| `output` | array of field shapes | Ordered output fields. | +| `output` | array of field shapes | Stable result contract. Models and clients must use these ordered field shapes. | Each field shape has `name` (string), `type` (string), and `required` (boolean). Input shapes use `str` and `int | None`. Output shapes use `str`, `int`, `bool`, and `float`. Output fields always report `required: true`. +`Description.Output` is the stable result contract. Clients that parsed a trailing ` -> ` suffix on `signature` must stop parsing it and use `output` instead. There is no compatibility suffix, alias, or alternate signature field. + #### `Execute` ```text diff --git a/internal/binding/signature.go b/internal/binding/signature.go index e547fe1..164e21f 100644 --- a/internal/binding/signature.go +++ b/internal/binding/signature.go @@ -16,7 +16,7 @@ type FieldShape struct { Required bool `json:"required"` } -// Signature renders the compact keyword-only model documentation from the immutable plan. +// Signature renders the compact keyword-only invocation form, ending at the closing parenthesis. func (plan *Plan) Signature(capabilityName string) string { var signature strings.Builder signature.WriteString(capabilityName) @@ -32,12 +32,7 @@ func (plan *Plan) Signature(capabilityName string) string { signature.WriteString(inputKindSignature(field.kind)) } } - signature.WriteString(") -> ") - if plan.outputType.Name() == "" { - signature.WriteString("object") - } else { - signature.WriteString(plan.outputType.Name()) - } + signature.WriteByte(')') return signature.String() } diff --git a/internal/binding/signature_test.go b/internal/binding/signature_test.go index e649424..c065c90 100644 --- a/internal/binding/signature_test.go +++ b/internal/binding/signature_test.go @@ -1,12 +1,25 @@ package binding import ( + "reflect" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +// ExportedOutput is a named exported result structure used only in signature tests. +type ExportedOutput struct { + // Name is a string output field. + Name string `json:"name"` +} + +// unexportedOutput is a named unexported result structure used only in signature tests. +type unexportedOutput struct { + // Name is a string output field. + Name string `json:"name"` +} + // TestSignatureUsesTheCompiledInputPlan proves documentation cannot drift from binding behavior. func TestSignatureUsesTheCompiledInputPlan(t *testing.T) { plan, err := CompileFor[representativeInput, representativeOutput]() @@ -14,7 +27,7 @@ func TestSignatureUsesTheCompiledInputPlan(t *testing.T) { signature := plan.Signature("records.lookup") - assert.Equal(t, "records.lookup(*, org: str, limit: int | None) -> representativeOutput", signature) + assert.Equal(t, "records.lookup(*, org: str, limit: int | None)", signature) } // TestSignatureHandlesAnEmptyInputStruct proves zero-argument capabilities omit the keyword-only marker. @@ -22,5 +35,38 @@ func TestSignatureHandlesAnEmptyInputStruct(t *testing.T) { plan, err := CompileFor[struct{}, representativeOutput]() require.NoError(t, err) - assert.Equal(t, "records.status() -> representativeOutput", plan.Signature("records.status")) + assert.Equal(t, "records.status()", plan.Signature("records.status")) +} + +// TestSignatureOmitsHostOutputTypeNames proves export status cannot change the invocation-only signature. +func TestSignatureOmitsHostOutputTypeNames(t *testing.T) { + const want = "records.lookup(*, org: str, limit: int | None)" + + tests := []struct { + // name identifies the output type export status. + name string + + // outputType is compiled as the capability result type. + outputType reflect.Type + + // outputName is the host Go identifier that must stay out of the signature. + outputName string + }{ + {name: "exported output", outputType: reflect.TypeFor[ExportedOutput](), outputName: "ExportedOutput"}, + {name: "unexported output", outputType: reflect.TypeFor[unexportedOutput](), outputName: "unexportedOutput"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + plan, err := Compile(reflect.TypeFor[representativeInput](), tt.outputType) + require.NoError(t, err) + + signature := plan.Signature("records.lookup") + + assert.Equal(t, want, signature) + assert.Equal(t, tt.outputName, plan.OutputType().Name()) + assert.NotContains(t, signature, tt.outputName) + assert.NotContains(t, signature, "->") + }) + } } diff --git a/internal/catalog/catalog_test.go b/internal/catalog/catalog_test.go index 615e388..487b6fe 100644 --- a/internal/catalog/catalog_test.go +++ b/internal/catalog/catalog_test.go @@ -27,6 +27,12 @@ type testOutput struct { Name string `json:"name"` } +// ExportedOutput is a registered exported result type whose identifier must stay host-local. +type ExportedOutput struct { + // Name is one result value. + Name string `json:"name"` +} + // TestBuildValidatesEveryRegistrationBeforeFiltering proves disabled entries cannot hide invalid contracts. func TestBuildValidatesEveryRegistrationBeforeFiltering(t *testing.T) { registration := validRegistration("cap.bad", "records.bad", "Bad record") @@ -183,7 +189,7 @@ func TestBuildFiltersOnceAndDerivesEverySurface(t *testing.T) { require.True(t, foundDescription) assert.Equal(t, "Alpha record", description.Summary) assert.Equal(t, "Alpha record full description.", description.Description) - assert.Equal(t, "records.alpha(*, org: str, limit: int | None) -> testOutput", description.Signature) + assert.Equal(t, "records.alpha(*, org: str, limit: int | None)", description.Signature) require.Len(t, description.Input, 2) assert.Equal(t, "org", description.Input[0].Name) assert.Equal(t, "str", description.Input[0].Type) @@ -222,7 +228,7 @@ func TestSearchIsSortedNormalizedAndBounded(t *testing.T) { require.NoError(t, err) require.Len(t, results, 2) assert.Equal(t, []string{"records.alpha", "records.beta"}, []string{results[0].Name, results[1].Name}) - assert.Equal(t, "records.alpha(*, org: str, limit: int | None) -> testOutput", results[0].Signature) + assert.Equal(t, "records.alpha(*, org: str, limit: int | None)", results[0].Signature) nonContiguous, err := catalog.Search("alpha record") require.NoError(t, err) @@ -241,6 +247,56 @@ func TestSearchIsSortedNormalizedAndBounded(t *testing.T) { require.ErrorIs(t, err, ErrSearchQueryLimit) } +// TestSearchAndDescribeOmitHostOutputTypeNames proves exported and unexported +// Go output identifiers never appear on catalog Search or Describe surfaces. +func TestSearchAndDescribeOmitHostOutputTypeNames(t *testing.T) { + catalog, err := Build([]Registration{ + validRegistration("cap.alpha", "records.alpha", "Alpha record"), + { + ID: "cap.status", + Name: "health.status", + Summary: "Health status", + Description: "Health status full description.", + Plan: mustCompileExportedOutputPlan(), + Invoke: func(context.Context, authz.Subject, any) (any, error) { + return ExportedOutput{Name: "ok"}, nil + }, + }, + }, testOptions()) + require.NoError(t, err) + + const unexportedName = "testOutput" + const exportedName = "ExportedOutput" + + results, err := catalog.Search("alpha") + require.NoError(t, err) + require.Len(t, results, 1) + assert.Equal(t, "records.alpha(*, org: str, limit: int | None)", results[0].Signature) + assertSearchOmitsOutputTypeNames(t, results[0], unexportedName, exportedName) + + description, found := catalog.Describe("records.alpha") + require.True(t, found) + assert.Equal(t, "records.alpha(*, org: str, limit: int | None)", description.Signature) + require.Len(t, description.Output, 1) + assert.Equal(t, "name", description.Output[0].Name) + assert.Equal(t, "str", description.Output[0].Type) + assertDescriptionOmitsOutputTypeNames(t, description, unexportedName, exportedName) + + statusResults, err := catalog.Search("health") + require.NoError(t, err) + require.Len(t, statusResults, 1) + assert.Equal(t, "health.status()", statusResults[0].Signature) + assertSearchOmitsOutputTypeNames(t, statusResults[0], unexportedName, exportedName) + + statusDescription, found := catalog.Describe("health.status") + require.True(t, found) + assert.Equal(t, "health.status()", statusDescription.Signature) + require.Len(t, statusDescription.Output, 1) + assert.Equal(t, "name", statusDescription.Output[0].Name) + assert.Equal(t, "str", statusDescription.Output[0].Type) + assertDescriptionOmitsOutputTypeNames(t, statusDescription, unexportedName, exportedName) +} + // TestCatalogSupportsConcurrentReadOnlyUse proves cloned views and immutable indexes are race-safe. func TestCatalogSupportsConcurrentReadOnlyUse(t *testing.T) { catalog, err := Build([]Registration{ @@ -329,3 +385,41 @@ func mustCompileTestPlan() *binding.Plan { } return plan } + +// mustCompileExportedOutputPlan compiles an empty input with an exported output type. +func mustCompileExportedOutputPlan() *binding.Plan { + plan, err := binding.CompileFor[struct{}, ExportedOutput]() + if err != nil { + panic(err) + } + return plan +} + +// assertSearchOmitsOutputTypeNames requires a Search result to omit host Go output identifiers. +func assertSearchOmitsOutputTypeNames(t *testing.T, result SearchResult, forbidden ...string) { + t.Helper() + for _, name := range forbidden { + assert.NotContains(t, result.Name, name) + assert.NotContains(t, result.Signature, name) + assert.NotContains(t, result.Summary, name) + } +} + +// assertDescriptionOmitsOutputTypeNames requires a Describe result to omit host Go output identifiers. +func assertDescriptionOmitsOutputTypeNames(t *testing.T, description Description, forbidden ...string) { + t.Helper() + for _, name := range forbidden { + assert.NotContains(t, description.Name, name) + assert.NotContains(t, description.Signature, name) + assert.NotContains(t, description.Summary, name) + assert.NotContains(t, description.Description, name) + for _, field := range description.Input { + assert.NotContains(t, field.Name, name) + assert.NotContains(t, field.Type, name) + } + for _, field := range description.Output { + assert.NotContains(t, field.Name, name) + assert.NotContains(t, field.Type, name) + } + } +} diff --git a/mcpserver/e2e_test.go b/mcpserver/e2e_test.go index 1715176..5c0d389 100644 --- a/mcpserver/e2e_test.go +++ b/mcpserver/e2e_test.go @@ -76,6 +76,12 @@ type lookupResult struct { Count int64 `json:"count"` } +// StatusResult is an exported handler output whose Go identifier must not cross MCP. +type StatusResult struct { + // State is the current status value. + State string `json:"state"` +} + // executeEnvelope is the exact structured execute payload. type executeEnvelope struct { // Result is main's final converted value. @@ -201,6 +207,15 @@ func TestActualMCPSecureLoop(t *testing.T) { return lookupResult{}, errors.New("disabled capability invoked") }, })) + require.NoError(t, codemode.Register(builder, codemode.Capability[struct{}, StatusResult]{ + ID: "health.entry.status", + Name: "health.status", + Summary: "Report current health.", + Description: "Returns one deterministic health status object.", + Handler: func(context.Context, authz.Subject, struct{}) (StatusResult, error) { + return StatusResult{State: "ok"}, nil + }, + })) root, err := builder.Build() require.NoError(t, err) @@ -240,8 +255,63 @@ func TestActualMCPSecureLoop(t *testing.T) { searchResults := decodeStructured[[]codemode.SearchResult](t, searched) require.Len(t, searchResults, 1) assert.Equal(t, "records.lookup", searchResults[0].Name) - assert.Equal(t, "records.lookup(*, key: str, limit: int | None) -> lookupResult", searchResults[0].Signature) + assert.Equal(t, "records.lookup(*, key: str, limit: int | None)", searchResults[0].Signature) assert.Equal(t, "Look up one record by key.", searchResults[0].Summary) + assertDiscoveryOmitsGoTypeNames(t, searched, "lookupResult", "StatusResult") + + described, err := session.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "describe_api", + Arguments: map[string]any{"name": "records.lookup"}, + }) + require.NoError(t, err) + assertSuccessfulTool(t, described) + assertNoCanary(t, described) + description := decodeStructured[codemode.Description](t, described) + assert.Equal(t, "records.lookup", description.Name) + assert.Equal(t, "records.lookup(*, key: str, limit: int | None)", description.Signature) + require.Len(t, description.Input, 2) + assert.Equal(t, "key", description.Input[0].Name) + assert.Equal(t, "str", description.Input[0].Type) + assert.True(t, description.Input[0].Required) + assert.Equal(t, "limit", description.Input[1].Name) + assert.Equal(t, "int | None", description.Input[1].Type) + assert.False(t, description.Input[1].Required) + require.Len(t, description.Output, 2) + assert.Equal(t, "key", description.Output[0].Name) + assert.Equal(t, "str", description.Output[0].Type) + assert.True(t, description.Output[0].Required) + assert.Equal(t, "count", description.Output[1].Name) + assert.Equal(t, "int", description.Output[1].Type) + assert.True(t, description.Output[1].Required) + assertDiscoveryOmitsGoTypeNames(t, described, "lookupResult", "StatusResult") + + statusSearch, err := session.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "search_api", + Arguments: map[string]any{"query": "health"}, + }) + require.NoError(t, err) + assertSuccessfulTool(t, statusSearch) + assertNoCanary(t, statusSearch) + statusResults := decodeStructured[[]codemode.SearchResult](t, statusSearch) + require.Len(t, statusResults, 1) + assert.Equal(t, "health.status", statusResults[0].Name) + assert.Equal(t, "health.status()", statusResults[0].Signature) + assertDiscoveryOmitsGoTypeNames(t, statusSearch, "lookupResult", "StatusResult") + + statusDescribed, err := session.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "describe_api", + Arguments: map[string]any{"name": "health.status"}, + }) + require.NoError(t, err) + assertSuccessfulTool(t, statusDescribed) + assertNoCanary(t, statusDescribed) + statusDescription := decodeStructured[codemode.Description](t, statusDescribed) + assert.Equal(t, "health.status", statusDescription.Name) + assert.Equal(t, "health.status()", statusDescription.Signature) + assert.Empty(t, statusDescription.Input) + require.Len(t, statusDescription.Output, 1) + assert.Equal(t, "state", statusDescription.Output[0].Name) + assertDiscoveryOmitsGoTypeNames(t, statusDescribed, "lookupResult", "StatusResult") hidden, err := session.CallTool(t.Context(), &mcp.CallToolParams{ Name: "describe_api", @@ -422,3 +492,18 @@ func assertNotContainsText(t *testing.T, result *mcp.CallToolResult, forbidden . assert.NotContains(t, payload, value) } } + +// assertDiscoveryOmitsGoTypeNames requires search/describe structured content +// and its JSON text mirror to omit host Go output identifiers. +func assertDiscoveryOmitsGoTypeNames(t *testing.T, result *mcp.CallToolResult, forbidden ...string) { + t.Helper() + structured, err := json.Marshal(result.StructuredContent) + require.NoError(t, err) + require.Len(t, result.Content, 1) + text, ok := result.Content[0].(*mcp.TextContent) + require.True(t, ok, "successful discovery content must be text") + for _, name := range forbidden { + assert.NotContains(t, string(structured), name) + assert.NotContains(t, text.Text, name) + } +} diff --git a/mcpserver/server_test.go b/mcpserver/server_test.go index 78f3a1b..aa7cdd2 100644 --- a/mcpserver/server_test.go +++ b/mcpserver/server_test.go @@ -165,7 +165,7 @@ func TestToolsResolveSubjectBeforeServiceWork(t *testing.T) { service.EXPECT().Search("lookup").RunAndReturn(func(string) ([]codemode.SearchResult, error) { events = append(events, "search") return []codemode.SearchResult{ - {Name: "records.lookup", Signature: "records.lookup() -> object", Summary: "lookup"}, + {Name: "records.lookup", Signature: "records.lookup()", Summary: "lookup"}, }, nil }).Once() service.EXPECT(). @@ -174,7 +174,7 @@ func TestToolsResolveSubjectBeforeServiceWork(t *testing.T) { events = append(events, "describe") return codemode.Description{ Name: "records.lookup", - Signature: "records.lookup() -> object", + Signature: "records.lookup()", Summary: "lookup", }, nil }). @@ -201,7 +201,7 @@ func TestToolsResolveSubjectBeforeServiceWork(t *testing.T) { require.False(t, describeResult.IsError) assert.Equal(t, map[string]any{ "name": "records.lookup", - "signature": "records.lookup() -> object", + "signature": "records.lookup()", "summary": "lookup", "description": "", "input": nil,