-
Notifications
You must be signed in to change notification settings - Fork 2
Description
Summary
While building a composite tool that calls multiple GitHub MCP tools (parallel PR data gathering + conditional review creation with an elicitation gate), we hit several issues that aren't covered in the composite tools guide.
Gaps found
1. JSON text responses vs structuredContent — no guidance on when to use fromJson
The guide's fromJson example shows extracting a single field, but doesn't explain how to determine whether a backend tool returns data in structuredContent fields directly or as JSON text in structuredContent.text. The only way to find out is to call the tool and inspect the raw response. The GitHub MCP server returns all data as JSON text, so every output reference needs .output.text rather than .output.fieldName.
Suggestion: Add a section explaining how to inspect raw responses and decide between direct access vs fromJson .steps.X.output.text.
2. Passing through whole arrays/objects breaks with fromJson
Using fromJson to parse an entire array or object and assigning it to an output property (e.g. value: "{{fromJson .steps.pr_files.output.text}}" with type: array) fails because the Go template engine stringifies the parsed result using Go's native format ([map[sha:... filename:...]]), not JSON. The output engine then can't deserialize it.
The workaround is to pass the raw .output.text as type: string instead.
Suggestion: Document the distinction between using fromJson for field extraction (scalar values) vs passing through entire responses (use raw .output.text as a string). The current example only shows field extraction.
3. Elicitation onDecline defaults to aborting the workflow
When a user declines an elicitation, the workflow fails with: elicitation request was declined by user: step <id>. The onDecline and onCancel fields with action: continue are needed to allow the workflow to proceed to a conditional step, but these aren't mentioned in the composite tools guide.
Suggestion: Add an example showing the elicitation → conditional step pattern with onDecline: action: continue.
4. defaultResults must match the downstream access pattern
When a conditional step is skipped and downstream output uses fromJson .steps.X.output.text, the defaultResults must provide a text field containing valid JSON — not bare key/value pairs. Otherwise fromJson fails because .output.text doesn't exist.
For example, this doesn't work:
defaultResults:
status: "skipped"
message: "User declined"This does:
defaultResults:
text: '{"state":"skipped","id":0}'Suggestion: Document that defaultResults must mirror the structure that downstream template expressions expect.
5. Null values can't coerce to typed output fields
Setting "id": null in defaultResults JSON and referencing it in an output property with type: integer fails with: cannot coerce "<no value>" to integer. Using 0 instead of null works.
Suggestion: Note that Go template null values (<no value>) can't be coerced to typed output properties, and recommend using zero values (0, "", false) instead of null.
Context
These were found while building a prepare_pr_review composite tool for a conference demo. The tool gathers PR metadata, diff, files, reviews, and review comments in parallel, then uses an elicitation to optionally create a pending review.