Correlate upstream 400s with request tool surface and negotiate MCP protocol version - #8861
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Advertised 2025-03-26 support lacks required batch handling, and malformed empty tool names evade the new diagnostic.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (3)
What changed in this PR
Adds diagnostics for upstream 400 responses and broadens enclave MCP protocol negotiation.
Changes:
- Logs bounded tool names/counts with upstream errors.
- Negotiates three MCP protocol revisions.
- Adds tests and documentation.
| File | Description |
|---|---|
containers/api-proxy/README.md |
Documents upstream diagnostics. |
containers/api-proxy/upstream-log.js |
Adds tool-summary log fields. |
containers/api-proxy/upstream-response.js |
Extracts request tool metadata. |
containers/api-proxy/upstream-response.test.js |
Tests tool diagnostics. |
containers/api-proxy/upstream-retry.js |
Propagates summaries through 400 handling. |
containers/enclave/mcp-server/mcp-protocol.js |
Adds protocol negotiation. |
docs/enclaves-architecture.md |
Documents supported revisions. |
src/enclave/mcp-server.test.ts |
Tests protocol negotiation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const SUPPORTED_MCP_PROTOCOL_VERSIONS = Object.freeze([ | ||
| '2025-11-25', | ||
| '2025-06-18', | ||
| '2025-03-26', |
|
|
||
| ## Upstream error diagnostics | ||
|
|
||
| Every non-2xx upstream response is logged as an `upstream_error_response` |
| function extractToolName(tool) { | ||
| if (!tool || typeof tool !== 'object') return null; | ||
| // OpenAI/Copilot chat-completions shape: { type: 'function', function: { name } } | ||
| if (tool.function && typeof tool.function === 'object' && typeof tool.function.name === 'string') { | ||
| return tool.function.name; | ||
| } | ||
| // Responses API and Anthropic shape: { type: 'function'|..., name } | ||
| if (typeof tool.name === 'string') return tool.name; | ||
| return null; | ||
| } |
|
@copilot address review feedback |
|
✅ Copilot review passed with no inline comments. @copilot Add the |
Documentation PreviewDocumentation has been built for this PR. To view locally:
Built from commit 5ffda84 |
|
🔌 Smoke Services — All services reachable! ✅
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤
|
|
❌ Smoke Gemini reports failed. Facets need polishing...
|
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded.
|
|
❌ Smoke Copilot BYOK AOAI (api-key) reports failed. AOAI BYOK (api-key) mode investigation needed...
|
|
✅ Smoke Claude passed Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "api.anthropic.com"See Network Configuration for more information.
|
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅
|
|
✅ Build Test Suite completed successfully!
|
|
✅ Security Guard completed successfully! Security review of PR #8861 complete. No security-weakening changes found. The PR adds tool surface diagnostics for error correlation and MCP protocol version negotiation — both changes maintain security boundaries with proper input validation and data sanitization.
|
|
🛡️ Smoke Copilot Network Isolation confirmed the egress allowlist is enforced. ✅ Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 Warning Firewall blocked 7 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "accounts.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "msfeed25.pkgs.visualstudio.com"
- "update.googleapis.com"
- "www.google.com"
- "www.gstatic.com"See Network Configuration for more information.
|
|
Smoke Cloud Hypervisor completed. Cloud Hypervisor + Copilot passed. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
Coverage comparison generated by |
Smoke Test: Cloud Hypervisor + Copilot
All 4 checks passed. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
Smoke Test: Claude Engine Validation
Overall result: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "api.anthropic.com"See Network Configuration for more information.
|
|
EGRESS_RESULT allow=pass deny=pass
Overall: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
github-actions
Bot
added
smoke-cloud-hypervisor
smoke-claude
smoke-copilot-network-isolation
Smoke Test: Copilot BYOK (Direct) Mode✅ PASS — All tests successful
Status: Direct BYOK mode (COPILOT_PROVIDER_API_KEY) operational
|
|
Smoke Test: Copilot Engine —
Overall: PASS
|
📡 Smoke Test: API Proxy OpenTelemetry Tracing — Results
Overall: All tracing scenarios validated. No regressions detected in OTEL span creation, GenAI token attributes, parent-context propagation, or env-var wiring.
|
|
Smoke Test: Services Connectivity
Overall: PASS
|
Chroot Version Comparison Results
Overall: FAILED — Node.js version mismatch between host and chroot environments.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS Note: Maven's default local repository ( All 18 projects across all 8 ecosystems built/installed and passed their test suites successfully through the AWF firewall proxy.
|
|
#8854 Sync D14 runner-doctor catalog with #8801 Cloud Hypervisor retry-fix follow-up Warning Firewall blocked 7 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "accounts.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "msfeed25.pkgs.visualstudio.com"
- "update.googleapis.com"
- "www.google.com"
- "www.gstatic.com"See Network Configuration for more information.
|
Copilot CLI 1.0.83 requests produce immediate zero-token
400s. Two gaps block root-causing it: theupstream_error_responsediagnostic records the failing response but nothing about the tool definitions that were sent, andenclave-mcp-serveranswersinitializewith a hardcoded protocol revision regardless of what the client negotiated (1.0.83 negotiates2025-11-25).api-proxy: request tool-surface diagnostics
summarizeRequestTools()(upstream-response.js) parses the request body'stoolsarray, handling the chat-completions ({type:'function', function:{name}}) and Responses/Anthropic ({name}) shapes.upstream_error_responsevia both error paths — the buffered-400 retry path (upstream-retry.js) and the streaming/piped path — as:{ "event": "upstream_error_response", "status": 400, "model": "gpt-5.6-sol", "request_tool_count": 3, "request_tool_names": ["github-list_issues", "safeoutputs-create_issue"], "request_unnamed_tool_count": 1 }request_unnamed_tool_countis the signal for malformed tool entries, the suspected 400 trigger.enclave-mcp-server: protocol version negotiation
initializenow echoes the client's requested revision when it is one of2025-11-25,2025-06-18,2025-03-26, else returns the newest supported (2025-11-25, previously the fixed2025-06-18).initialize,tools/list,tools/call, which are identical across those revisions, so accepting all three is a wire-compatible widening rather than a semantics change.Docs
containers/api-proxy/README.mddocuments the new fields and the existingAWF_MAX_ERROR_RESPONSE_CAPTURE_BYTESbody capture.docs/enclaves-architecture.mddocuments the negotiated revision set.The
-32022rejections in the original report come from thegithubandsafeoutputsMCP servers, which live ingh-aw; this change fixes AWF's own server and supplies the payload correlation needed to confirm or rule out the MCP causal chain for the rest.