Skip to content

Commit 0bfed9a

Browse files
Mark conditional schema parameters in generated docs
Previously `show_ui` was listed in docs/feature-flags.md and docs/insiders-features.md alongside ordinary parameters with no indication that it is hidden from clients without MCP App UI support. A reader scanning the parameter list would assume it is always available. Add a programmatic conditional-property mechanism: - `inventory.ConditionalSchemaPropertyDescriptions()` exposes a map[propertyName]conditionDescription derived from the same uiOnlySchemaProperties allowlist that drives the per-request strip in ToolsForRegistration. Single source of truth. - The doc generator (writeToolDoc) consults this map and appends "conditional — <description>" to the parameter's parenthesised type/required suffix. Example rendered output: - `show_ui`: Whether to render the MCP App form... (boolean, optional, conditional — only visible to clients that advertise MCP App UI support) A small test (TestConditionalSchemaPropertyDescriptions) ensures every entry in uiOnlySchemaProperties has a description, so a future stripped property addition can't silently lose its doc marker. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 0f2ad87 commit 0bfed9a

5 files changed

Lines changed: 44 additions & 5 deletions

File tree

cmd/github-mcp-server/generate_docs.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,8 @@ func writeToolDoc(buf *strings.Builder, tool inventory.ServerTool) {
257257
}
258258
sort.Strings(paramNames)
259259

260+
conditional := inventory.ConditionalSchemaPropertyDescriptions()
261+
260262
for i, propName := range paramNames {
261263
prop := schema.Properties[propName]
262264
required := slices.Contains(schema.Required, propName)
@@ -282,7 +284,11 @@ func writeToolDoc(buf *strings.Builder, tool inventory.ServerTool) {
282284
// Indent any continuation lines in the description to maintain markdown formatting
283285
description := indentMultilineDescription(prop.Description, " ")
284286

285-
fmt.Fprintf(buf, " - `%s`: %s (%s, %s)", propName, description, typeStr, requiredStr)
287+
if cond, isConditional := conditional[propName]; isConditional {
288+
fmt.Fprintf(buf, " - `%s`: %s (%s, %s, conditional — %s)", propName, description, typeStr, requiredStr, cond)
289+
} else {
290+
fmt.Fprintf(buf, " - `%s`: %s (%s, %s)", propName, description, typeStr, requiredStr)
291+
}
286292
if i < len(paramNames)-1 {
287293
buf.WriteString("\n")
288294
}

docs/feature-flags.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ runtime behavior (such as output formatting) won't appear here.
4444
- `maintainer_can_modify`: Allow maintainer edits (boolean, optional)
4545
- `owner`: Repository owner (string, required)
4646
- `repo`: Repository name (string, required)
47-
- `show_ui`: Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when you have all required values (especially ones the form does not collect, like reviewers) and the user has already confirmed the action. (boolean, optional)
47+
- `show_ui`: Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when you have all required values (especially ones the form does not collect, like reviewers) and the user has already confirmed the action. (boolean, optional, conditional — only visible to clients that advertise MCP App UI support)
4848
- `title`: PR title (string, required)
4949

5050
- **get_me** - Get my user profile
@@ -67,7 +67,7 @@ runtime behavior (such as output formatting) won't appear here.
6767
- `milestone`: Milestone number (number, optional)
6868
- `owner`: Repository owner (string, required)
6969
- `repo`: Repository name (string, required)
70-
- `show_ui`: Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when you have all required values (especially ones the form does not collect, like labels, assignees, milestone, type, or state changes) and the user has already confirmed the action. (boolean, optional)
70+
- `show_ui`: Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when you have all required values (especially ones the form does not collect, like labels, assignees, milestone, type, or state changes) and the user has already confirmed the action. (boolean, optional, conditional — only visible to clients that advertise MCP App UI support)
7171
- `state`: New state (string, optional)
7272
- `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional)
7373
- `title`: Issue title (string, optional)

docs/insiders-features.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ The list below is generated from the Go source. It covers tool **inventory and s
3838
- `maintainer_can_modify`: Allow maintainer edits (boolean, optional)
3939
- `owner`: Repository owner (string, required)
4040
- `repo`: Repository name (string, required)
41-
- `show_ui`: Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when you have all required values (especially ones the form does not collect, like reviewers) and the user has already confirmed the action. (boolean, optional)
41+
- `show_ui`: Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when you have all required values (especially ones the form does not collect, like reviewers) and the user has already confirmed the action. (boolean, optional, conditional — only visible to clients that advertise MCP App UI support)
4242
- `title`: PR title (string, required)
4343

4444
- **get_me** - Get my user profile
@@ -61,7 +61,7 @@ The list below is generated from the Go source. It covers tool **inventory and s
6161
- `milestone`: Milestone number (number, optional)
6262
- `owner`: Repository owner (string, required)
6363
- `repo`: Repository name (string, required)
64-
- `show_ui`: Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when you have all required values (especially ones the form does not collect, like labels, assignees, milestone, type, or state changes) and the user has already confirmed the action. (boolean, optional)
64+
- `show_ui`: Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when you have all required values (especially ones the form does not collect, like labels, assignees, milestone, type, or state changes) and the user has already confirmed the action. (boolean, optional, conditional — only visible to clients that advertise MCP App UI support)
6565
- `state`: New state (string, optional)
6666
- `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional)
6767
- `title`: Issue title (string, optional)

pkg/inventory/builder.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,22 @@ var uiOnlySchemaProperties = []string{
418418
"show_ui", // explicit "render the MCP App form" toggle on form-backed write tools
419419
}
420420

421+
// ConditionalSchemaPropertyDescriptions returns a map of schema property name
422+
// to a human-readable description of the condition under which the property
423+
// is visible to clients. The doc generator uses this to annotate conditional
424+
// parameters so readers can see at a glance which fields are not always
425+
// available. This is the single source of truth for the conditional-property
426+
// surface — entries here must correspond to a strip rule in
427+
// ToolsForRegistration.
428+
func ConditionalSchemaPropertyDescriptions() map[string]string {
429+
const uiOnlyCondition = "only visible to clients that advertise MCP App UI support"
430+
out := make(map[string]string, len(uiOnlySchemaProperties))
431+
for _, name := range uiOnlySchemaProperties {
432+
out[name] = uiOnlyCondition
433+
}
434+
return out
435+
}
436+
421437
// stripUIOnlySchemaProperties removes UI-capability-gated input-schema
422438
// properties (currently just "show_ui") from each tool's static schema.
423439
// Tools whose InputSchema is not a *jsonschema.Schema (e.g. json.RawMessage)

pkg/inventory/registry_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2182,6 +2182,23 @@ func TestStripUIOnlySchemaProperties(t *testing.T) {
21822182
"tools with a non-*jsonschema.Schema input schema must be passed through")
21832183
}
21842184

2185+
// TestConditionalSchemaPropertyDescriptions ensures every property that
2186+
// inventory strips per-request also has a human-readable condition the doc
2187+
// generator can render. A future addition to uiOnlySchemaProperties that
2188+
// forgets to wire a description through will fail here.
2189+
func TestConditionalSchemaPropertyDescriptions(t *testing.T) {
2190+
t.Parallel()
2191+
2192+
descs := ConditionalSchemaPropertyDescriptions()
2193+
require.NotEmpty(t, descs, "expected at least show_ui to be advertised as conditional")
2194+
2195+
for _, name := range uiOnlySchemaProperties {
2196+
desc, ok := descs[name]
2197+
require.Truef(t, ok, "ui-only property %q must have a conditional description", name)
2198+
require.NotEmptyf(t, desc, "conditional description for %q must be non-empty", name)
2199+
}
2200+
}
2201+
21852202
func TestToolsForRegistration_StripsShowUIUnderSameGate(t *testing.T) {
21862203
// A tool whose schema declares both `_meta.ui` and `show_ui`. The strip
21872204
// for both must fire — or not — together, governed by the same gate

0 commit comments

Comments
 (0)