Repository navigation
Support move semantics in mcp_write_result and optimize MCP notifications and headers - #3025
HarshPopat23 wants to merge 1 commit into
Conversation
🤖 Augment PR SummaryAdds move-aware MCP result writing and reduces redundant MCP work.
🤖 Was this summary useful? React with 👍 or 👎 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 6 files
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
d768565 to
0e08943
Compare
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
|
augment review |
0e08943 to
8b1f2c7
Compare
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| const std::optional<MCPImplementation> &server_info) | ||
| -> void { | ||
| const auto envelope_identifier{identifier}; | ||
| decorate_result(version, result, server_info); |
There was a problem hiding this comment.
Could you check the remaining borrowed inputs at src/core/mcp/results.cc:1180–1182 when they alias result? Adding resultType can reallocate the object's storage, leaving a short-string method view (e.g. result.at("method").to_string() containing "tools/list") or server_info.icons pointing to a top-level array dangling before decorate_cacheable_result/attach_server_info reads it, causing a use-after-free.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
8b1f2c7 to
b9078a6
Compare
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
View guided diff | Re-trigger cubic
b9078a6 to
2fa1c09
Compare
…ions and headers - Provide an overload mcp_write_result(..., JSON &&result, ...) in mcp_results.h and results.cc to eliminate deep clones of multi-megabyte payloads when callers pass an rvalue. Validate results prior to copying in the lvalue wrapper, and copy or materialize borrowed inputs (identifiers, methods, server information) before in-place decoration. - In notifications.cc, resolve the log level enum once into a local variable instead of parsing twice and unconditionally dereferencing mcp_resolve_log_level. - In parameter_headers.cc, eliminate repeated temporary JSON allocations for JavaScript safe integer boundaries by using scalar comparisons for integers and static constants for integral fallback. - Add unit tests verifying move semantics, safe integer boundary validations, and aliased inputs. Signed-off-by: HarshPopat23 <musichk61@gmail.com>
2fa1c09 to
cf4d34e
Compare
This branch has not been deployed
1 participant
Summary
This PR addresses three performance and safety optimizations in the Model Context Protocol (MCP) implementation:
Move Semantics for MCP Result Writing:
esources/read\ or \ ools/list\ payloads.
Eliminate Duplicate Log Level Enum Resolution:
esolved_level\ when checking \mcp_log_level_enabled, avoiding redundant lookups and unverified pointer dereferences.
Avoid Temporary JSON Allocations in Parameter Headers:
Testing