feat(ENG-733): support flat_prefixed MCP param style - #192
Conversation
There was a problem hiding this comment.
Pull request overview
This PR stabilizes MCP tool schemas and RPC execution by pinning the /mcp tool-listing param style to flat_prefixed and adding an argument “envelope” splitter that routes <location>_<field> keys into the correct RPC buckets (path, query, headers, body), while preserving backward-compat for nested envelopes.
Changes:
- Pin
/mcptool listing to?param-style=flat_prefixedso LLM schemas remain stable across server default changes. - Replace the RPC argument unwrapping logic with
_StackOneRpcTool._split_envelope_paramsto supportflat_prefixedtool arguments. - Add targeted tests for param-style pinning and envelope splitting behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
stackone_ai/toolset.py |
Pins MCP listing param-style and introduces _split_envelope_params used by _StackOneRpcTool.execute() to correctly bucket flat-prefixed args into the RPC envelope. |
tests/test_fetch_tools.py |
Adds tests asserting the pinned param-style in /mcp URL construction and validating envelope splitting for flat-prefixed + nested/unprefixed keys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return buckets | ||
|
|
||
| def _build_action_headers(self, additional_headers: dict[str, Any] | None) -> dict[str, str]: |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="stackone_ai/toolset.py">
<violation number="1" location="stackone_ai/toolset.py:482">
P2: The existing `tests/test_tool_calling.py` suite now fails because `_extract_record` was removed. Please retain a compatibility implementation (or update/remove the stale tests as part of this change) so the repository test suite does not raise `AttributeError`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if isinstance(value, dict): | ||
| return dict(value) | ||
| return None | ||
| def _split_envelope_params(params: dict[str, Any]) -> dict[str, dict[str, Any]]: |
There was a problem hiding this comment.
P2: The existing tests/test_tool_calling.py suite now fails because _extract_record was removed. Please retain a compatibility implementation (or update/remove the stale tests as part of this change) so the repository test suite does not raise AttributeError.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At stackone_ai/toolset.py, line 482:
<comment>The existing `tests/test_tool_calling.py` suite now fails because `_extract_record` was removed. Please retain a compatibility implementation (or update/remove the stale tests as part of this change) so the repository test suite does not raise `AttributeError`.</comment>
<file context>
@@ -477,10 +479,28 @@ def _parse_arguments(self, arguments: str | dict[str, Any] | None) -> dict[str,
- if isinstance(value, dict):
- return dict(value)
- return None
+ def _split_envelope_params(params: dict[str, Any]) -> dict[str, dict[str, Any]]:
+ """Split LLM-supplied tool arguments into the RPC envelope (path/query/headers/body).
+
</file context>
1aebf53 to
d84f094
Compare
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
Problem
The SDK lists tools from
/mcpwithout pinning a param-style, so the tool schema handed to the LLM follows the server default. Execution, however, goes through/actions/rpcwith a hardcoded nested unwrap (path/query/headers/body). When the server default param-style flipsnested→flat_prefixed, the LLM emitspath_id/query_limit/… which the nested unwrap can't route: path and query params are dropped and the prefixed keys leak into the request body. The server-side MCP shim does not cover this — SDK execution bypasses the MCP transport and hits the actions RPC endpoint directly.Fix
?param-style=flat_prefixedon the/mcptool-listing endpoint, so the schema the LLM sees is stable regardless of the server default._extract_recordunwrap with_split_envelope_params, which buckets<location>_<field>keys back intopath/query/headers/body. The prefix carries the location, so no per-action schema is needed. Bare nested envelopes are still accepted (for clients holding a cached nested schema), and unprefixed keys fall through to the body.Net effect: SDK tool calls use the higher-accuracy flat_prefixed schema and are immune to the server-side default flip.
Tests
test_fetch_tools_pins_flat_prefixed_param_styletest_routes_flat_prefixed_keys_by_locationtest_buckets_nested_envelope_and_unprefixed_keystytype-check andruff(lint + format) clean.Note
This protects SDK versions from this release forward. Already-published versions still break the moment the server default flips, so the server-side flip should be gated on quantifying old-version exposure by User-Agent.
Summary by cubic
Support the
flat_prefixedMCP param style end-to-end so tool execution stays correct even if the server default changes. Addresses ENG-733 by pinning the/mcpschema and aligning RPC argument handling./mcp?param-style=flat_prefixedso tools expose<location>_<field>args consistently; tests assert the exact endpoint._split_envelope_paramsto bucketpath_/query_/headers_/body_-prefixed keys into the RPC envelope; still accepts nested dicts and treats unprefixed keys as body for backward compatibility.Written for commit b895af1. Summary will update on new commits.