Add MCP Gateway payload path prefix and size threshold configuration#18617
Add MCP Gateway payload path prefix and size threshold configuration#18617
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
@claude[agent] fix the PR title and summary |
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Updated PR title to "Add MCP Gateway payload path prefix and size threshold configuration" with a detailed description of the changes. See commit 2729392. |
|
@copilot flatten the frontmatter fields: github: |
|
@copilot integrity -> min-integrity in frontmatter |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added tools:
github:
repos:
- owner/repo1
- owner/repo2
min-integrity: sha256:abc123...
Used Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
…fig" This reverts commit edf34c1. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Reverted in commit 7b6a167. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
This comment has been minimized.
This comment has been minimized.
🤖 Contribution Check — PR #18617Hey Checklist Results
Verdict: 🟢 AlignedThis PR looks solid and ready for maintainer review once the draft status is lifted. The implementation is clean, tests are thorough, and documentation is well-written.
|
There was a problem hiding this comment.
Pull request overview
This pull request adds two new configuration fields to the MCP Gateway for improved payload handling in containerized agent environments: PayloadPathPrefix (for Docker-in-Docker path remapping) and PayloadSizeThreshold (with a 512KB default to prevent agent looping issues).
Changes:
- Added
DefaultMCPGatewayPayloadSizeThresholdconstant (512KB) and two new fields (PayloadPathPrefix,PayloadSizeThreshold) toMCPGatewayRuntimeConfig - Updated gateway config builder, JSON renderer, and MCP setup generator to handle new fields with appropriate defaults
- Updated all golden test workflow files and Codex engine test to reflect new configuration format
Reviewed changes
Copilot reviewed 172 out of 172 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/constants/constants.go | Adds 512KB default threshold constant with documentation |
| pkg/workflow/tools_types.go | Extends MCPGatewayRuntimeConfig struct with two new optional fields |
| pkg/workflow/mcp_gateway_config.go | Populates new fields in gateway config with defaults |
| pkg/workflow/mcp_renderer.go | Updates JSON rendering to include optional payload fields |
| pkg/workflow/mcp_setup_generator.go | Exports environment variables for new configuration fields |
| pkg/workflow/codex_engine_test.go | Updates test expectations to include payloadSizeThreshold |
| .github/workflows/*.lock.yml | Updates all golden test files with new environment variables and JSON fields |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pkg/workflow/mcp_renderer.go
Outdated
| needsComma := true | ||
| if options.GatewayConfig.PayloadDir != "" { | ||
| fmt.Fprintf(&configBuilder, ",\n \"payloadDir\": \"%s\"\n", options.GatewayConfig.PayloadDir) | ||
| } else { | ||
| configBuilder.WriteString("\n") | ||
| if needsComma { | ||
| configBuilder.WriteString(",") | ||
| } | ||
| fmt.Fprintf(&configBuilder, "\n \"payloadDir\": \"%s\"", options.GatewayConfig.PayloadDir) | ||
| } | ||
| if options.GatewayConfig.PayloadPathPrefix != "" { | ||
| if needsComma || options.GatewayConfig.PayloadDir != "" { | ||
| configBuilder.WriteString(",") | ||
| } | ||
| fmt.Fprintf(&configBuilder, "\n \"payloadPathPrefix\": \"%s\"", options.GatewayConfig.PayloadPathPrefix) | ||
| } | ||
| if options.GatewayConfig.PayloadSizeThreshold > 0 { | ||
| if needsComma || options.GatewayConfig.PayloadDir != "" || options.GatewayConfig.PayloadPathPrefix != "" { | ||
| configBuilder.WriteString(",") | ||
| } | ||
| fmt.Fprintf(&configBuilder, "\n \"payloadSizeThreshold\": %d", options.GatewayConfig.PayloadSizeThreshold) |
There was a problem hiding this comment.
The comma handling logic for optional fields is incorrect. The needsComma variable is set to true on line 957 but is never used after that, as subsequent conditions check both needsComma and whether previous fields were set. This creates redundant logic and could lead to incorrect comma placement. Since apiKey always exists and ends without a comma on line 954, all subsequent optional fields need a comma. The logic should simply check if any field has been added before adding a comma, or remove the needsComma variable entirely and use a simpler approach.
|
@copilot apply changes based on the comments in this thread |
Remove the always-true needsComma variable and simplify each optional field to unconditionally prepend a comma, since apiKey always precedes them without a trailing comma. Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Fixed in commit a89e8d2. Removed the |
#18728) * Initial plan * Initial plan: update wasm golden test files Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * Update wasm golden test files to fix build-wasm CI failure Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
…18735) * Initial plan * Fix action_pins tests: add missing checkout@v5.0.1, setup-dotnet@v4.3.1, upload-artifact@v5.0.0 entries Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
) * Initial plan * Fix failing tests by updating action SHA pins to latest versions Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
needsCommalogic inmcp_renderer.go— simplify optional field comma handling✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.