diff --git a/SPECS/ARCHIVE/FU-P13-T7_Fix_structuredContent_compliance_for_empty_content_tool_results/FU-P13-T7_Fix_structuredContent_compliance_for_empty_content_tool_results.md b/SPECS/ARCHIVE/FU-P13-T7_Fix_structuredContent_compliance_for_empty_content_tool_results/FU-P13-T7_Fix_structuredContent_compliance_for_empty_content_tool_results.md new file mode 100644 index 0000000..489e0de --- /dev/null +++ b/SPECS/ARCHIVE/FU-P13-T7_Fix_structuredContent_compliance_for_empty_content_tool_results/FU-P13-T7_Fix_structuredContent_compliance_for_empty_content_tool_results.md @@ -0,0 +1,106 @@ +# PRD: FU-P13-T7 — Enforce strict `structuredContent` compliance for empty-content tool results + +**Created:** 2026-02-16 +**Priority:** P0 +**Branch:** feature/FU-P13-T7-structuredcontent-compliance +**Status:** PLAN + +--- + +## Problem Statement + +Strict MCP clients (Cursor, Codex) validate that every `tools/call` response includes a +`structuredContent` field. When Xcode MCP bridge returns `result.content: []` without +`result.structuredContent`, these clients raise a schema violation and may abort the session. + +--- + +## Current State + +The core injection logic **is already present** (implemented in P4-T1 / BUG-T5): + +- `needs_transformation()` returns `True` for `content: []` (no `structuredContent`) +- `inject_structured_content()` injects `structuredContent: {}` for empty arrays +- Basic unit tests cover this path (`test_empty_content_array_gets_empty_structured_content`) + +**Gap 1 — Missing edge-case coverage:** No test covers `tools/call` with `isError=true` +and `content: []` together. In this scenario, `normalize_resources_error` skips the +response (method == `tools/call`), then `inject_structured_content` injects `{}` as +expected — but this flow has no dedicated regression test. + +**Gap 2 — Missing documentation:** `troubleshooting.md` has no section explaining the +`structuredContent` compliance requirement for strict clients, so users hitting this +error have no guidance. + +--- + +## Deliverables + +| # | Artifact | Description | +|---|----------|-------------| +| 1 | `tests/unit/test_transform.py` | Add targeted tests for `isError=true` + `content:[]` on `tools/call`; add test verifying non-tool notifications are unaffected | +| 2 | `docs/troubleshooting.md` | Add section: "Tool has empty content but no structuredContent (strict MCP clients)" | + +No changes to `src/mcpbridge_wrapper/transform.py` — the implementation is already +correct and complete. + +--- + +## Acceptance Criteria + +- [x] For tool responses missing `structuredContent`, empty `content` results are + normalized to include `structuredContent` fallback *(already implemented)* +- [x] Existing already-compliant responses remain unchanged *(covered by existing tests)* +- [x] Non-tool JSON-RPC notifications and unrelated payloads are not regressed + *(covered by existing tests)* +- [ ] New unit tests fail before fix and pass after fix — *applied as: new tests + added to prevent future regression; they pass on current codebase which contains + the fix* +- [ ] `docs/troubleshooting.md` section added for strict-client empty-content behavior + +--- + +## Test Plan + +### New tests in `TestProcessResponseLine` + +1. `test_tools_call_iserror_true_empty_content_gets_structured_content` + Input: `{"jsonrpc":"2.0","id":1,"result":{"isError":true,"content":[]}}` + Method: `tools/call` + Expected: `result.structuredContent == {}` + +2. `test_non_tool_notification_no_result_unchanged` + Input: `{"jsonrpc":"2.0","method":"notifications/initialized","params":{}}` + Method: `None` + Expected: line unchanged (no result → no transformation) + +3. `test_empty_content_response_roundtrip_with_other_fields` + Verify `id`, `jsonrpc`, `isError` fields are preserved after injection. + +--- + +## Docs Update Plan + +Add a new subsection to `docs/troubleshooting.md` under **Common Errors**: + +``` +### "Tool has output schema but did not return structured content" (empty result) + +**Symptom:** Cursor/Codex reports a schema violation even though the tool ran successfully. + +**Cause:** The Xcode MCP bridge returned `result.content: []` (empty) without a +`structuredContent` field. Strict MCP clients require `structuredContent` on every +tool response. + +**Solution:** mcpbridge-wrapper automatically injects `structuredContent: {}` for +empty-content tool responses. Ensure you are using mcpbridge-wrapper (not connecting +directly to `xcrun mcpbridge`). +``` + +--- + +## Dependencies + +- P3-T3 ✅ (transformation engine) +- P4-T1 ✅ (empty-content skip logic / inject) +- P5-T6 ✅ (process_response_line integration) diff --git a/SPECS/ARCHIVE/FU-P13-T7_Fix_structuredContent_compliance_for_empty_content_tool_results/FU-P13-T7_Validation_Report.md b/SPECS/ARCHIVE/FU-P13-T7_Fix_structuredContent_compliance_for_empty_content_tool_results/FU-P13-T7_Validation_Report.md new file mode 100644 index 0000000..7f4826e --- /dev/null +++ b/SPECS/ARCHIVE/FU-P13-T7_Fix_structuredContent_compliance_for_empty_content_tool_results/FU-P13-T7_Validation_Report.md @@ -0,0 +1,54 @@ +# Validation Report: FU-P13-T7 + +**Task:** Fix_structuredContent_compliance_for_empty_content_tool_results +**Date:** 2026-02-16 +**Verdict:** PASS + +--- + +## Implementation Summary + +The core empty-content `structuredContent` injection logic was already implemented in +`transform.py` (as part of P4-T1 / BUG-T5). This task added: + +1. **6 new targeted regression tests** in a new `TestEmptyContentStrictCompliance` class + covering: + - `tools/call isError=true` + `content:[]` gets `structuredContent: {}` + - Field preservation after injection + - JSON-RPC notifications (no `result`) pass through unchanged + - Success empty-content roundtrip + - Already-compliant responses not overwritten + +2. **Troubleshooting docs update** in `docs/troubleshooting.md`: new subsection + "Tool has output schema but did not return structured content (empty result)" + explaining strict-client behavior and the wrapper's automatic fix. + +--- + +## Quality Gates + +| Gate | Result | +|------|--------| +| `pytest tests/` | ✅ 471 passed, 5 skipped | +| `ruff check src/` | ✅ All checks passed | +| `pytest --cov` | ✅ 95.6% (≥90% required) | +| mypy | N/A (not configured) | + +--- + +## Acceptance Criteria + +| Criterion | Status | +|-----------|--------| +| Empty `content` results normalized to include `structuredContent` fallback | ✅ Already implemented, verified by tests | +| Existing already-compliant responses remain unchanged | ✅ Verified by `test_already_compliant_empty_content_not_overwritten` | +| Non-tool notifications and unrelated payloads not regressed | ✅ Verified by `test_notification_without_result_is_unchanged`, `test_notification_with_method_arg_is_unchanged` | +| New unit tests pass on current codebase | ✅ 6 new tests all PASS | +| `docs/troubleshooting.md` section added | ✅ Added | + +--- + +## Files Changed + +- `tests/unit/test_transform.py` — added `TestEmptyContentStrictCompliance` class (6 tests) +- `docs/troubleshooting.md` — added strict-client empty-result troubleshooting section diff --git a/SPECS/ARCHIVE/INDEX.md b/SPECS/ARCHIVE/INDEX.md index 083c88d..eb0974e 100644 --- a/SPECS/ARCHIVE/INDEX.md +++ b/SPECS/ARCHIVE/INDEX.md @@ -102,6 +102,7 @@ | FU-P11-T2-2 | [FU-P11-T2-2_Add_limit_query_param_to_GET_api_sessions/](FU-P11-T2-2_Add_limit_query_param_to_GET_api_sessions/) | 2026-02-16 | PASS | | FU-P11-T1-1 | [FU-P11-T1-1_Refactor_FakeWebUIConfig_to_MagicMock/](FU-P11-T1-1_Refactor_FakeWebUIConfig_to_MagicMock/) | 2026-02-16 | PASS | | FU-P12-T2-1 | [FU-P12-T2-1_Fix_stacking_click_listeners_in_updateLatencyTable/](FU-P12-T2-1_Fix_stacking_click_listeners_in_updateLatencyTable/) | 2026-02-16 | PASS | +| FU-P13-T7 | [FU-P13-T7_Fix_structuredContent_compliance_for_empty_content_tool_results/](FU-P13-T7_Fix_structuredContent_compliance_for_empty_content_tool_results/) | 2026-02-16 | PASS | ## Historical Artifacts @@ -167,6 +168,7 @@ | [REVIEW_FU-P11-T2-2_limit_query_param.md](_Historical/REVIEW_FU-P11-T2-2_limit_query_param.md) | Review report for FU-P11-T2-2 | | [REVIEW_FU-P11-T1-1_fake_webuiconfig_refactor.md](_Historical/REVIEW_FU-P11-T1-1_fake_webuiconfig_refactor.md) | Review report for FU-P11-T1-1 | | [REVIEW_FU-P12-T2-1_stacking_click_listeners.md](_Historical/REVIEW_FU-P12-T2-1_stacking_click_listeners.md) | Review report for FU-P12-T2-1 | +| [REVIEW_FU-P13-T7_structuredcontent_compliance.md](_Historical/REVIEW_FU-P13-T7_structuredcontent_compliance.md) | Review report for FU-P13-T7 | ## Archive Log @@ -288,3 +290,5 @@ | 2026-02-16 | FU-P11-T1-1 | Archived REVIEW_FU-P11-T1-1_fake_webuiconfig_refactor report | | 2026-02-16 | FU-P12-T2-1 | Archived Fix_stacking_click_listeners_in_updateLatencyTable (PASS) | | 2026-02-16 | FU-P12-T2-1 | Archived REVIEW_FU-P12-T2-1_stacking_click_listeners report | +| 2026-02-16 | FU-P13-T7 | Archived Fix_structuredContent_compliance_for_empty_content_tool_results (PASS) | +| 2026-02-16 | FU-P13-T7 | Archived REVIEW_FU-P13-T7_structuredcontent_compliance report | diff --git a/SPECS/ARCHIVE/_Historical/REVIEW_FU-P13-T7_structuredcontent_compliance.md b/SPECS/ARCHIVE/_Historical/REVIEW_FU-P13-T7_structuredcontent_compliance.md new file mode 100644 index 0000000..e881145 --- /dev/null +++ b/SPECS/ARCHIVE/_Historical/REVIEW_FU-P13-T7_structuredcontent_compliance.md @@ -0,0 +1,61 @@ +## REVIEW REPORT — FU-P13-T7: structuredContent Compliance for Empty-Content Tool Results + +**Scope:** origin/main..HEAD +**Files:** 2 (tests/unit/test_transform.py, docs/troubleshooting.md) +**Date:** 2026-02-16 + +--- + +### Summary Verdict + +- [x] Approve + +--- + +### Critical Issues + +None. + +--- + +### Secondary Issues + +None. + +--- + +### Architectural Notes + +- The task confirmed that the core fix (`structuredContent: {}` injection for + `content: []`) was already implemented in `transform.py` as part of P4-T1/BUG-T5. + FU-P13-T7 added the missing regression test coverage and documentation. + +- The `TestEmptyContentStrictCompliance` test class is correctly scoped: it tests the + end-to-end `process_response_line` path (including the interaction between + `normalize_resources_error` skipping `tools/call` and `inject_structured_content` + running), not just individual functions in isolation. + +- The notification passthrough tests (`test_notification_without_result_is_unchanged`, + `test_notification_with_method_arg_is_unchanged`) provide explicit regression + coverage ensuring the transformation pipeline is side-effect-free for non-result + JSON-RPC messages — previously not explicitly tested. + +- The troubleshooting doc correctly distinguishes two distinct symptoms with similar + wording: the existing entry covers "direct bridge connection" and the new entry + covers "empty result from wrapper". The proximity of these entries in the docs is + appropriate since users may arrive at the same symptom string. + +--- + +### Tests + +- 6 new tests in `TestEmptyContentStrictCompliance` — all pass. +- Total test suite: 471 passed, 5 skipped. +- Coverage: 95.6% (≥90% required). No regressions introduced. +- Ruff: clean. + +--- + +### Next Steps + +No actionable issues found. FOLLOW-UP skipped. diff --git a/SPECS/INPROGRESS/next.md b/SPECS/INPROGRESS/next.md index f1ba38c..f30625f 100644 --- a/SPECS/INPROGRESS/next.md +++ b/SPECS/INPROGRESS/next.md @@ -4,14 +4,14 @@ The previously selected task has been archived. ## Recently Archived +- 2026-02-16 — FU-P13-T7: Enforce strict `structuredContent` compliance for empty-content tool results (PASS) - 2026-02-16 — FU-P12-T2-1: Fix stacking click event listeners in `updateLatencyTable` (PASS) - 2026-02-16 — FU-P11-T1-1: Refactor `_FakeWebUIConfig` test stub to use `MagicMock(spec=WebUIConfig)` (PASS) - 2026-02-16 — FU-P11-T2-2: Add `limit` query param to `GET /api/sessions` (PASS) - 2026-02-16 — FU-P11-T2-1: Push session data via WebSocket (PASS) -- 2026-02-16 — P12-T2: Add Tool Parameter Frequency Analysis (PASS) ## Suggested Next Tasks -- FU-P12-T1-1: Remove or document `MCPInitializeParams` in schemas (P3, depends on P12-T1 ✅) -- FU-P12-T3-2: Add `error_code` column to audit CSV export (P3, depends on P12-T3 ✅) -- FU-P12-T1-2: Add code comment clarifying stdin-only client capture in `on_request` (P3, depends on P12-T1 ✅) +- FU-P13-T8: Prevent Web UI port collision from destabilizing MCP sessions (P0, parallelizable) +- P13-T1: Design persistent broker architecture and protocol contract (P0, Phase 13 foundation) +- FU-P12-T1-1: Remove or document `MCPInitializeParams` in schemas (P3) diff --git a/SPECS/Workplan.md b/SPECS/Workplan.md index e34ddba..778efe1 100644 --- a/SPECS/Workplan.md +++ b/SPECS/Workplan.md @@ -1122,7 +1122,7 @@ Tool has output schema but did not return structured content Use clients/builds with compatibility fallback behavior. This is not reliable for strict validation paths. #### Resolution Path -- [ ] Implement FU-P13-T7 +- [x] Implement FU-P13-T7 - [ ] Add strict empty-content regression tests - [ ] Verify behavior in Codex App and Codex CLI with same wrapper binary @@ -1955,7 +1955,7 @@ Phase 9 Follow-up Backlog --- -#### FU-P13-T7: Enforce strict `structuredContent` compliance for empty-content tool results +#### ✅ FU-P13-T7: Enforce strict `structuredContent` compliance for empty-content tool results - **Description:** Fix transformation logic so strict MCP clients no longer fail when a tool response includes `result.content: []` without `result.structuredContent`. Add a fallback injection strategy for transformable tool results with empty content. - **Priority:** P0 - **Dependencies:** P3-T3, P4-T1, P5-T6 diff --git a/Sources/XcodeMCPWrapper/Documentation.docc/Troubleshooting.md b/Sources/XcodeMCPWrapper/Documentation.docc/Troubleshooting.md index ce99386..0cf6718 100644 --- a/Sources/XcodeMCPWrapper/Documentation.docc/Troubleshooting.md +++ b/Sources/XcodeMCPWrapper/Documentation.docc/Troubleshooting.md @@ -32,6 +32,33 @@ Found 0 tools, 0 prompts, and 0 resources 2. Not `xcrun mcpbridge` directly 3. See for configuration +## Error: "Tool has output schema but did not return structured content" (empty result) + +**Symptom:** Cursor or Codex reports a schema violation even though the tool ran successfully +and returned an empty result. + +**Cause:** The Xcode MCP bridge returned `result.content: []` (empty content array) +without a `structuredContent` field. Strict MCP clients require `structuredContent` +on every `tools/call` response, even when the content list is empty. + +**Solution:** mcpbridge-wrapper automatically injects `structuredContent: {}` for +empty-content tool responses. Ensure you are connecting through the wrapper: + +```bash +# Correct — via wrapper +uvx mcpbridge-wrapper + +# Incorrect — direct bridge connection (no transformation) +xcrun mcpbridge +``` + +If you are already using the wrapper and still see this error, verify you are on +version 0.5.0 or later: + +```bash +uvx mcpbridge-wrapper --version +``` + ## Error: "command not found: uvx" **Symptom:** uvx command not found when using the recommended installation method diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 0547655..40df151 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -35,11 +35,43 @@ This confirms the issue is with Xcode settings, not the wrapper. **Cause:** You're connecting directly to `xcrun mcpbridge` without the wrapper. -**Solution:** +**Solution:** 1. Ensure your MCP client is configured to use the wrapper via **uvx** or `xcodemcpwrapper` 2. Not `xcrun mcpbridge` directly 3. See [Cursor Setup](cursor-setup.md) for configuration +### "Tool has output schema but did not return structured content" (empty result) + +**Symptom:** Cursor or Codex reports a schema violation such as: + +``` +Tool has output schema but did not return structured content +``` + +even though the tool ran successfully and returned an empty result. + +**Cause:** The Xcode MCP bridge returned `result.content: []` (empty content array) +without a `structuredContent` field. Strict MCP clients require `structuredContent` +on every `tools/call` response, even when the content list is empty. + +**Solution:** mcpbridge-wrapper automatically injects `structuredContent: {}` for +empty-content tool responses. Ensure you are connecting through the wrapper: + +```bash +# Correct — via wrapper +uvx mcpbridge-wrapper + +# Incorrect — direct bridge connection (no transformation) +xcrun mcpbridge +``` + +If you are already using the wrapper and still see this error, verify you are on +version 0.5.0 or later: + +```bash +uvx mcpbridge-wrapper --version +``` + ### "command not found: uvx" **Symptom:** uvx command not found when using the recommended installation method diff --git a/tests/unit/test_transform.py b/tests/unit/test_transform.py index a332122..aaf3351 100644 --- a/tests/unit/test_transform.py +++ b/tests/unit/test_transform.py @@ -806,3 +806,87 @@ def test_non_json_line_unchanged_with_method(self) -> None: line = "Some plain log output\n" result = process_response_line(line, method="resources/list") assert result == line + + +class TestEmptyContentStrictCompliance: + """Regression tests for strict structuredContent compliance with empty-content results. + + Strict MCP clients (Cursor, Codex) require structuredContent on every tools/call + response. These tests guard the empty-content injection path against regressions. + """ + + def test_tools_call_iserror_empty_content_gets_structured_content(self) -> None: + """tools/call isError=true with content:[] must still get structuredContent injected.""" + line = json.dumps( + { + "jsonrpc": "2.0", + "id": 1, + "result": {"isError": True, "content": []}, + } + ) + result = process_response_line(line, method="tools/call") + parsed = json.loads(result) + # normalize_resources_error skips tools/call — inject_structured_content runs + assert "result" in parsed + assert "error" not in parsed + assert parsed["result"]["structuredContent"] == {} + assert parsed["result"]["isError"] is True + + def test_tools_call_iserror_empty_content_preserves_all_fields(self) -> None: + """All top-level fields are preserved after structuredContent injection.""" + line = json.dumps( + { + "jsonrpc": "2.0", + "id": 42, + "result": {"isError": True, "content": []}, + } + ) + result = process_response_line(line, method="tools/call") + parsed = json.loads(result) + assert parsed["jsonrpc"] == "2.0" + assert parsed["id"] == 42 + assert parsed["result"]["structuredContent"] == {} + + def test_notification_without_result_is_unchanged(self) -> None: + """JSON-RPC notifications (no result field) pass through untouched.""" + line = json.dumps({"jsonrpc": "2.0", "method": "notifications/initialized", "params": {}}) + result = process_response_line(line, method=None) + assert result == line + parsed = json.loads(result) + assert "result" not in parsed + assert "structuredContent" not in parsed + + def test_notification_with_method_arg_is_unchanged(self) -> None: + """Notifications are unchanged even when a method context is provided.""" + line = json.dumps( + {"jsonrpc": "2.0", "method": "notifications/tools/list_changed", "params": {}} + ) + result = process_response_line(line, method="tools/list") + assert result == line + + def test_tools_call_success_empty_content_roundtrip(self) -> None: + """tools/call success with empty content still satisfies strict structuredContent.""" + line = json.dumps( + { + "jsonrpc": "2.0", + "id": 7, + "result": {"content": []}, + } + ) + result = process_response_line(line, method="tools/call") + parsed = json.loads(result) + assert parsed["result"]["structuredContent"] == {} + assert parsed["result"]["content"] == [] + + def test_already_compliant_empty_content_not_overwritten(self) -> None: + """structuredContent is not overwritten when already present on empty content.""" + line = json.dumps( + { + "jsonrpc": "2.0", + "id": 8, + "result": {"content": [], "structuredContent": {"custom": True}}, + } + ) + result = process_response_line(line, method="tools/call") + parsed = json.loads(result) + assert parsed["result"]["structuredContent"] == {"custom": True}