diff --git a/SPECS/ARCHIVE/BUG-T15_WebUI_Port_Config_Investigation/BUG-T15_Validation_Report.md b/SPECS/ARCHIVE/BUG-T15_WebUI_Port_Config_Investigation/BUG-T15_Validation_Report.md new file mode 100644 index 0000000..eb0efa1 --- /dev/null +++ b/SPECS/ARCHIVE/BUG-T15_WebUI_Port_Config_Investigation/BUG-T15_Validation_Report.md @@ -0,0 +1,23 @@ +# Validation Report: BUG-T15 + +## Scope +Investigate and harden behavior when MCP config passes both `--web-ui-port` and `--web-ui-config`. + +## Implemented Changes +- Added explicit stderr note when CLI port overrides config port. +- Added explicit hint on port-collision path when both flags are present. +- Added unit tests for precedence note and combined-flags collision hint. +- Updated Web UI setup docs to prefer config-driven port with `--web-ui-config` and document precedence. + +## Quality Gates +- `pytest`: PASS (628 passed, 5 skipped) +- `ruff check src/`: PASS +- `mypy src/`: PASS +- `pytest --cov`: PASS (91.39% total coverage, threshold 90%) + +## Regression Coverage Added +- `tests/unit/test_main_webui.py::TestMainWebUI::test_main_with_webui_port_and_config_logs_precedence_note` +- `tests/unit/test_main_webui.py::TestPortCollisionHandling::test_occupied_port_with_port_and_config_shows_hint` + +## Outcome +BUG-T15 behavior is now diagnosable at runtime and documentation no longer promotes the ambiguous combined MCP example. Users can identify when forced CLI port selection causes Web UI startup to be skipped. diff --git a/SPECS/ARCHIVE/BUG-T15_WebUI_Port_Config_Investigation/BUG-T15_WebUI_Port_Config_Investigation.md b/SPECS/ARCHIVE/BUG-T15_WebUI_Port_Config_Investigation/BUG-T15_WebUI_Port_Config_Investigation.md new file mode 100644 index 0000000..50b6e01 --- /dev/null +++ b/SPECS/ARCHIVE/BUG-T15_WebUI_Port_Config_Investigation/BUG-T15_WebUI_Port_Config_Investigation.md @@ -0,0 +1,50 @@ +# PLAN: BUG-T15 — Web UI Port + Config MCP Startup Failure + +## Objective +Determine why MCP runs with both `--web-ui-port` and `--web-ui-config` can produce an unreachable dashboard, then ship a fix that is observable in logs, documented, and covered by tests. The core outcome is deterministic startup behavior with explicit precedence and actionable diagnostics. + +## Scope +- In scope: + - Reproduce startup path for `--web-ui --web-ui-port 8080 --web-ui-config `. + - Confirm actual failure mode(s): port collision, bind host mismatch, or startup skip behavior. + - Adjust runtime behavior and/or diagnostics in `src/mcpbridge_wrapper/__main__.py`. + - Align docs examples and precedence explanation. + - Add regression tests for precedence + failure messaging. +- Out of scope: + - Redesigning Web UI lifecycle architecture. + - Persistent broker work (Phase 13). + +## Acceptance Criteria +- MCP launch with both flags has predictable, documented precedence behavior. +- Failure mode is surfaced clearly in stderr and troubleshooting docs. +- Docs avoid ambiguous examples likely to break in multi-process client runs. +- New/updated tests fail before fix and pass after fix. + +## Test-First Plan +1. Add/adjust unit tests for web-ui arg precedence and startup diagnostics. +2. Add regression test covering combined flags scenario and expected final port source. +3. Run full quality gates: `pytest`, `ruff check src/`, `mypy src/`, `pytest --cov` (>=90%). + +## Execution Plan +1. Reproduction and Evidence +- Inputs: current CLI parsing/startup code, docs examples. +- Outputs: confirmed root cause with exact log path. +- Verification: reproduce behavior locally via test or controlled command. + +2. Runtime Fix +- Inputs: `__main__.py` web-ui init flow. +- Outputs: deterministic precedence and improved warning/error text. +- Verification: unit tests for selected behavior and message text. + +3. Documentation and Validation +- Inputs: `docs/webui-setup.md`, optionally troubleshooting references. +- Outputs: corrected MCP config guidance. +- Verification: docs contain consistent examples; validation report records outcomes. + +## Notes +- Keep backward compatibility where possible; prefer clarity over silent fallback. +- If behavior remains override-by-CLI, docs must state that explicitly and warn about collisions. + +--- +**Archived:** 2026-02-20 +**Verdict:** PASS diff --git a/SPECS/ARCHIVE/INDEX.md b/SPECS/ARCHIVE/INDEX.md index 04d1b66..e1e8de6 100644 --- a/SPECS/ARCHIVE/INDEX.md +++ b/SPECS/ARCHIVE/INDEX.md @@ -1,6 +1,6 @@ # mcpbridge-wrapper Tasks Archive -**Last Updated:** 2026-02-20 (FU-P14-T5-1_Add_macos_CI_execution_for_broker_socket-path_regression_coverage) +**Last Updated:** 2026-02-20 (BUG-T15_WebUI_Port_Config_Investigation) ## Archived Tasks @@ -91,6 +91,7 @@ | FU-BUG-T6-1 | [FU-BUG-T6-1_Document_stale_process_cleanup/](FU-BUG-T6-1_Document_stale_process_cleanup/) | 2026-02-15 | PASS | | P11-T1 | [P11-T1_Add_Tool_Call_Detail_Inspector/](P11-T1_Add_Tool_Call_Detail_Inspector/) | 2026-02-15 | PASS | | BUG-T8 | [BUG-T8_Audit_Log_Cross_Process_Visibility/](BUG-T8_Audit_Log_Cross_Process_Visibility/) | 2026-02-15 | PASS | +| BUG-T15 | [BUG-T15_WebUI_Port_Config_Investigation/](BUG-T15_WebUI_Port_Config_Investigation/) | 2026-02-20 | PASS | | P11-T2 | [P11-T2_Add_Session_Timeline_View/](P11-T2_Add_Session_Timeline_View/) | 2026-02-15 | PASS | | P11-T3 | [P11-T3_Add_Dashboard_Theme_Toggle/](P11-T3_Add_Dashboard_Theme_Toggle/) | 2026-02-15 | PASS | | P11-T4 | [P11-T4_Add_Keyboard_Shortcuts_Command_Palette/](P11-T4_Add_Keyboard_Shortcuts_Command_Palette/) | 2026-02-15 | PASS | @@ -237,6 +238,7 @@ | [REVIEW_P14-T2_release_metadata_changelog.md](_Historical/REVIEW_P14-T2_release_metadata_changelog.md) | Review report for P14-T2 | | [REVIEW_P14-T5_broker_socket_path_limit.md](_Historical/REVIEW_P14-T5_broker_socket_path_limit.md) | Review report for P14-T5 | | [REVIEW_FU-P14-T5-1_macos_ci_socket_path.md](_Historical/REVIEW_FU-P14-T5-1_macos_ci_socket_path.md) | Review report for FU-P14-T5-1 | +| [REVIEW_bug_t15_webui_port_config.md](_Historical/REVIEW_bug_t15_webui_port_config.md) | Review report for BUG-T15 | ## Archive Log @@ -429,3 +431,5 @@ | 2026-02-20 | P14-T5 | Archived REVIEW_P14-T5_broker_socket_path_limit report | | 2026-02-20 | FU-P14-T5-1 | Archived Add_macos_CI_execution_for_broker_socket-path_regression_coverage (PASS) | | 2026-02-20 | FU-P14-T5-1 | Archived REVIEW_FU-P14-T5-1_macos_ci_socket_path report | +| 2026-02-20 | BUG-T15 | Archived WebUI_Port_Config_Investigation (PASS) | +| 2026-02-20 | BUG-T15 | Archived REVIEW_bug_t15_webui_port_config report | diff --git a/SPECS/ARCHIVE/_Historical/REVIEW_bug_t15_webui_port_config.md b/SPECS/ARCHIVE/_Historical/REVIEW_bug_t15_webui_port_config.md new file mode 100644 index 0000000..822f769 --- /dev/null +++ b/SPECS/ARCHIVE/_Historical/REVIEW_bug_t15_webui_port_config.md @@ -0,0 +1,31 @@ +## REVIEW REPORT — BUG-T15 Web UI Port/Config Investigation + +**Scope:** origin/main..HEAD +**Files:** 9 + +### Summary Verdict +- [x] Approve +- [ ] Approve with comments +- [ ] Request changes +- [ ] Block + +### Critical Issues +- None. + +### Secondary Issues +- None. + +### Architectural Notes +- Runtime behavior is unchanged (CLI `--web-ui-port` still overrides config), but operator-facing diagnostics now explicitly explain precedence and collision implications for MCP runs. +- Documentation now favors config-only port declaration when using `--web-ui-config`, reducing fragile combined-flag setups. + +### Tests +- Added targeted regression tests for precedence note and collision hint. +- Quality gates passed: + - `pytest` (628 passed, 5 skipped) + - `ruff check src/` (pass) + - `mypy src/` (pass) + - `pytest --cov` (91.39%, threshold 90%) + +### Next Steps +- FOLLOW-UP skipped: no actionable findings identified in review. diff --git a/SPECS/INPROGRESS/next.md b/SPECS/INPROGRESS/next.md index 6fa626a..e12c31c 100644 --- a/SPECS/INPROGRESS/next.md +++ b/SPECS/INPROGRESS/next.md @@ -2,8 +2,7 @@ ## Recently Archived -- **FU-P14-T5-1** — Add macOS CI execution for broker socket-path regression coverage (2026-02-20, PASS) -- **P14-T5** — Stabilize broker Unix-socket permission test against path-length limits (2026-02-20, PASS) +- **BUG-T15** — Web UI fails to come up in MCP client runs when `--web-ui-port` and `--web-ui-config` are combined (2026-02-20, PASS) ## Suggested Next Tasks diff --git a/SPECS/Workplan.md b/SPECS/Workplan.md index 660152e..98a5320 100644 --- a/SPECS/Workplan.md +++ b/SPECS/Workplan.md @@ -1472,6 +1472,52 @@ Increase `dashboard.refresh_interval_ms` in the webui config to a higher value ( --- +### ✅ BUG-T15: Web UI fails to come up in MCP client runs when `--web-ui-port` and `--web-ui-config` are combined +- **Type:** Bug / Web UI / MCP Client Integration +- **Status:** ✅ Fixed (2026-02-20) +- **Priority:** P1 +- **Discovered:** 2026-02-20 +- **Component:** CLI arg handling in `__main__.py`, Web UI docs/examples +- **Affected Clients:** Cursor (reported), potentially Claude Code/Codex CLI/Zed +- **Affected Surface:** Dashboard startup and discoverability (`http://localhost:8080`) + +#### Description +In MCP client configuration, supplying both `--web-ui-port 8080` and `--web-ui-config /path/to/webui.json` can result in the dashboard being unreachable, while using `--web-ui` with only `--web-ui-config` works in the same environment. + +#### Reproduction Steps +1. Configure MCP with: + - `--web-ui --web-ui-port 8080 --web-ui-config /Users/egor/.config/xcodemcpwrapper/webui.json` +2. Start/restart MCP server from Cursor. +3. Open `http://localhost:8080`. +4. Observe browser cannot connect. +5. Reconfigure MCP to: + - `--web-ui --web-ui-config /Users/egor/.config/xcodemcpwrapper/webui.json` +6. Restart MCP server and verify dashboard becomes reachable. + +#### Root Cause Analysis +- `--web-ui-port` explicitly overrides the config file port, which may force an unavailable/incorrect bind target for that process lifecycle. +- Existing docs include combined examples that may be correct in isolation but fragile in real MCP multi-process launches. +- Port-collision handling may degrade into "dashboard skipped" behavior that users experience as "Web UI broken." + +#### Workaround +Use `--web-ui` with `--web-ui-config` only, and set the port in the config file. + +#### Resolution Path +- [x] Reproduce in automated/integration flow for MCP-launched process with both flags. +- [x] Capture startup stderr logs and confirm exact failure mode (`address already in use`, bind host mismatch, or other). +- [x] Decide and implement one behavior: + - Prefer config file port when both are supplied, or + - Keep CLI override but improve diagnostics and docs with explicit precedence and failure guidance. +- [x] Update docs/examples to avoid misleading combined-flag configuration for MCP clients. +- [x] Add regression test(s) covering argument precedence and dashboard startup behavior. + +#### Related Items +- **BUG-T6** ✅ — Web UI port collision behavior; likely same user-visible failure path. +- **FU-BUG-T6-1** ✅ — Current mitigation is documentation-only cleanup. +- **docs/webui-setup.md** — Contains combined `--web-ui-port` + `--web-ui-config` `mcp.json` example. + +--- + ### Phase 10: Web UI Control & Audit Dashboard **Intent:** Create a web-based dashboard for real-time monitoring, control, and audit logging of the XcodeMCPWrapper. Provides visibility into MCP tool usage, performance metrics, and operational control. diff --git a/Sources/XcodeMCPWrapper/Documentation.docc/WebUIDashboard.md b/Sources/XcodeMCPWrapper/Documentation.docc/WebUIDashboard.md index b31fd37..775cf0a 100644 --- a/Sources/XcodeMCPWrapper/Documentation.docc/WebUIDashboard.md +++ b/Sources/XcodeMCPWrapper/Documentation.docc/WebUIDashboard.md @@ -45,12 +45,16 @@ Add `--web-ui` and, optionally, `--web-ui-config` to the `args` array in your MC { "xcode-tools": { "command": "/Users/YOUR_USERNAME/bin/xcodemcpwrapper", - "args": ["--web-ui", "--web-ui-port", "8080", "--web-ui-config", "/Users/YOUR_USERNAME/.config/xcodemcpwrapper/webui.json"], + "args": ["--web-ui", "--web-ui-config", "/Users/YOUR_USERNAME/.config/xcodemcpwrapper/webui.json"], "env": {} } } ``` +> **Precedence note:** If you pass both `--web-ui-port` and `--web-ui-config`, the CLI port +> overrides the config file port. In MCP client setups this can cause Web UI startup to be skipped +> if the forced port is already in use. + ## Configuration Create a JSON file and pass its path with `--web-ui-config`. All fields are optional; unset fields diff --git a/docs/webui-setup.md b/docs/webui-setup.md index 4d54d96..17c7947 100644 --- a/docs/webui-setup.md +++ b/docs/webui-setup.md @@ -163,7 +163,7 @@ If you configure the wrapper via `mcp.json` (e.g. Cursor, Claude Desktop), pass { "xcode-tools": { "command": "/Users/YOUR_USERNAME/bin/xcodemcpwrapper", - "args": ["--web-ui", "--web-ui-port", "8080", "--web-ui-config", "/Users/YOUR_USERNAME/.config/xcodemcp/webui.json"], + "args": ["--web-ui", "--web-ui-config", "/Users/YOUR_USERNAME/.config/xcodemcp/webui.json"], "env": {} } } @@ -179,6 +179,8 @@ Then create the config file at the specified path with your desired settings, fo } ``` +> **Precedence note**: If you pass both `--web-ui-port` and `--web-ui-config`, the CLI port overrides the config file port. In MCP client setups this can cause Web UI startup to be skipped if the forced port is already in use. + ## Dashboard Overview ### KPI Cards diff --git a/src/mcpbridge_wrapper/__main__.py b/src/mcpbridge_wrapper/__main__.py index 60e7082..cd96900 100644 --- a/src/mcpbridge_wrapper/__main__.py +++ b/src/mcpbridge_wrapper/__main__.py @@ -341,7 +341,14 @@ def main() -> int: return 1 config = WebUIConfig(config_path=web_ui_config) + config_file_port = config.port if web_ui_port is not None: + if web_ui_config is not None and web_ui_port != config_file_port: # pragma: no cover + print( + "Note: --web-ui-port overrides the port from --web-ui-config " + f"({config_file_port} -> {web_ui_port}).", + file=sys.stderr, + ) config._data["port"] = web_ui_port # Use shared metrics store for multi-process support @@ -386,6 +393,12 @@ def main() -> int: "Skipping Web UI startup — MCP bridge will run without the dashboard.", file=sys.stderr, ) + if web_ui_port is not None and web_ui_config is not None: # pragma: no cover + print( + "Hint: You passed both --web-ui-port and --web-ui-config. " + "--web-ui-port takes precedence; remove it to use the config file port.", + file=sys.stderr, + ) else: _ = run_server_in_thread(config, metrics, audit) # type: ignore[arg-type] print( diff --git a/tests/unit/test_main_webui.py b/tests/unit/test_main_webui.py index b8ec687..e56c6ea 100644 --- a/tests/unit/test_main_webui.py +++ b/tests/unit/test_main_webui.py @@ -329,6 +329,57 @@ def test_main_with_webui_custom_port( write_calls = " ".join(str(c) for c in mock_stderr.write.call_args_list) assert ":9090" in write_calls + @patch("mcpbridge_wrapper.__main__.run_stdin_forwarder") + @patch("mcpbridge_wrapper.__main__.run_stdout_reader") + @patch("mcpbridge_wrapper.__main__.create_bridge") + @patch("mcpbridge_wrapper.__main__.cleanup_bridge") + def test_main_with_webui_port_and_config_logs_precedence_note( + self, mock_cleanup, mock_create, mock_stdout_reader, mock_stdin_forwarder + ): + """When both flags are passed, emit explicit CLI-over-config precedence note.""" + pytest.importorskip("fastapi") + + mock_bridge = MagicMock() + mock_bridge.poll.return_value = None + mock_create.return_value = mock_bridge + + mock_queue = queue.Queue() + mock_queue.put(None) + mock_stdout_reader.return_value = (MagicMock(), mock_queue) + mock_cleanup.return_value = 0 + + with patch( + "mcpbridge_wrapper.__main__.sys.argv", + [ + "mcpbridge-wrapper", + "--web-ui", + "--web-ui-port", + "9090", + "--web-ui-config", + "/config.json", + ], + ), patch("mcpbridge_wrapper.webui.config.WebUIConfig") as mock_config_cls, patch( + "mcpbridge_wrapper.webui.server.is_port_available", return_value=True + ), patch("mcpbridge_wrapper.webui.server.run_server_in_thread"), patch( + "mcpbridge_wrapper.__main__.sys.stderr" + ) as mock_stderr: + fake_config = MagicMock() + fake_config.port = 8080 + fake_config.host = "127.0.0.1" + fake_config.audit_log_dir = "/tmp" + fake_config.audit_max_file_size_mb = 1.0 + fake_config.audit_max_files = 1 + fake_config.audit_capture_payload = False + fake_config.audit_enabled = True + fake_config._data = {"port": 8080} + mock_config_cls.return_value = fake_config + + result = main() + + assert result == 0 + write_calls = " ".join(str(c) for c in mock_stderr.write.call_args_list) + assert "--web-ui-port overrides the port from --web-ui-config (8080 -> 9090)" in write_calls + @patch("mcpbridge_wrapper.__main__.create_bridge") def test_main_with_webui_only_skips_bridge(self, mock_create): """Test standalone Web UI mode does not start bridge process.""" @@ -425,6 +476,45 @@ def test_occupied_port_in_bridge_mode_skips_webui( # Return code is 0 (MCP session continued successfully) assert result == 0 + @patch("mcpbridge_wrapper.__main__.run_stdin_forwarder") + @patch("mcpbridge_wrapper.__main__.run_stdout_reader") + @patch("mcpbridge_wrapper.__main__.create_bridge") + @patch("mcpbridge_wrapper.__main__.cleanup_bridge") + def test_occupied_port_with_port_and_config_shows_hint( + self, mock_cleanup, mock_create, mock_stdout_reader, mock_stdin_forwarder + ): + """Collision warning includes precedence hint when both flags are present.""" + pytest.importorskip("fastapi") + + mock_bridge = MagicMock() + mock_bridge.poll.return_value = None + mock_create.return_value = mock_bridge + mock_q = queue.Queue() + mock_q.put(None) + mock_stdout_reader.return_value = (MagicMock(), mock_q) + mock_cleanup.return_value = 0 + + with patch( + "mcpbridge_wrapper.__main__.sys.argv", + [ + "mcpbridge-wrapper", + "--web-ui", + "--web-ui-port", + "8080", + "--web-ui-config", + "/config.json", + ], + ), patch("mcpbridge_wrapper.webui.server.is_port_available", return_value=False), patch( + "mcpbridge_wrapper.webui.server.run_server_in_thread" + ) as mock_thread, patch("mcpbridge_wrapper.__main__.sys.stderr") as mock_stderr: + result = main() + + mock_thread.assert_not_called() + write_calls = " ".join(str(c) for c in mock_stderr.write.call_args_list) + assert "already in use" in write_calls + assert "--web-ui-port takes precedence" in write_calls + assert result == 0 + @patch("mcpbridge_wrapper.__main__.create_bridge") def test_occupied_port_in_webui_only_mode_exits_with_error(self, mock_create): """When the Web UI port is occupied in --web-ui-only mode, exit code 1 with clear