From f0943e2bf0211f1828d9d1685aa6183575eed7a6 Mon Sep 17 00:00:00 2001 From: Egor Merkushev Date: Sat, 7 Mar 2026 18:18:30 +0300 Subject: [PATCH 1/8] Branch for P7-T3: improve dashboard port conflict recovery From 85a715050ea881b447050ac78e487eaa08664d6c Mon Sep 17 00:00:00 2001 From: Egor Merkushev Date: Sat, 7 Mar 2026 18:19:30 +0300 Subject: [PATCH 2/8] Select task P7-T3: Auto-recover or guide on dashboard port ownership conflicts --- SPECS/INPROGRESS/next.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/SPECS/INPROGRESS/next.md b/SPECS/INPROGRESS/next.md index f3d1a3b..062aa83 100644 --- a/SPECS/INPROGRESS/next.md +++ b/SPECS/INPROGRESS/next.md @@ -4,7 +4,7 @@ **Phase:** Phase 7: Broker UX and Diagnostics **Effort:** 4-6 hours **Dependencies:** P6-T1, P7-T2 -**Status:** Ready after `P7-T2` CI clears +**Status:** Selected ## Description @@ -20,5 +20,5 @@ required by the recommended UX flow. ## Next Step -Wait for the `P7-T2` pull request to clear CI, then run FLOW again to select -and plan `P7-T3`. +Create the `P7-T3` PRD in `SPECS/INPROGRESS/`, then implement and validate the +dashboard port-conflict recovery path. From cb032518f3dd5a6d40e1dfee342894d37c2a8355 Mon Sep 17 00:00:00 2001 From: Egor Merkushev Date: Sat, 7 Mar 2026 18:19:40 +0300 Subject: [PATCH 3/8] Plan task P7-T3: Auto-recover or guide on dashboard port ownership conflicts --- ...e_on_dashboard_port_ownership_conflicts.md | 149 ++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 SPECS/INPROGRESS/P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts.md diff --git a/SPECS/INPROGRESS/P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts.md b/SPECS/INPROGRESS/P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts.md new file mode 100644 index 0000000..0f644ce --- /dev/null +++ b/SPECS/INPROGRESS/P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts.md @@ -0,0 +1,149 @@ +# P7-T3 — Auto-recover or guide on dashboard port ownership conflicts + +## Objective Summary + +`P7-T1` introduced the explicit dedicated-host workflow and `P7-T2` gave users +one-command diagnostics, but the most confusing failure mode is still in the +startup path itself: `--broker-daemon --web-ui` can leave a running broker +behind while silently skipping the dashboard whenever the desired port is +occupied. That produces a partial state where the recommended frontend is down, +TUI cannot attach, and the user is forced to reason about multiple black boxes +at once. + +`P7-T3` should remove that ambiguity. When users request the broker-hosted +dashboard, startup must either recover deterministically into a usable state or +stop with one explicit remediation path. The implementation should align +broker-daemon startup, broker-console orchestration, doctor messaging, and TUI +feedback around the same model so the user is never told that a degraded +dashboard-less host is “good enough”. + +## Deliverables + +- Tighten the broker-daemon startup path in + `src/mcpbridge_wrapper/__main__.py` so an explicit `--web-ui` request does not + silently degrade into “broker without dashboard”. +- Add shared logic that classifies dashboard port conflicts into actionable + buckets: + - healthy broker-backed dashboard already serving the port + - foreign or stale listener on the desired port + - local broker already running without dashboard on the desired port + - restart-eligible wrapper-owned listener when `--web-ui-restart` is used +- Update user-facing messaging so the failing command prints one clear next + action instead of continuing in a partial state. +- Extend doctor/TUI-facing guidance where needed so conflict explanations and + remediation text stay consistent with the startup behavior. +- Add regression tests for broker-daemon, broker-console, and diagnostics + scenarios that previously stranded users. + +## Success Criteria + +- `mcpbridge-wrapper --broker-daemon --web-ui` no longer leaves a healthy-seeming + broker running without the requested dashboard when the port cannot be used. +- Port conflicts resolve into one of two outcomes only: + - the requested broker-backed dashboard becomes available, or + - the command exits non-zero with an explicit remediation path +- `--broker-console` and `--doctor` surface the same conflict class and + recommended next action for the same runtime state. +- Tests pin both safe recovery and fail-fast behavior so future changes do not + reintroduce the hidden partial state. + +## Test-First Plan + +1. Add broker-daemon CLI tests that lock in the new fail-fast behavior when + `--web-ui` is requested but the target port is occupied by a foreign + listener, stale listener, or unusable existing runtime. +2. Add orchestration tests for `--broker-console` that verify it reuses a + healthy broker-backed dashboard, restarts only when explicitly permitted, and + reports a single remediation path for foreign ownership conflicts. +3. Add doctor classification/rendering tests for the updated conflict buckets so + diagnostics remain aligned with the startup behavior. +4. Only after the new expectations are pinned, implement the shared conflict + classifier and wire it into the daemon startup/orchestration flow. +5. Run required quality gates: `pytest`, `ruff check src/`, `mypy src/`, and + `pytest --cov`. + +## Execution Plan + +### Phase 1: Define the startup contract + +Inputs: +- `src/mcpbridge_wrapper/__main__.py` +- existing broker-console/dashboard probe helpers +- `src/mcpbridge_wrapper/doctor.py` + +Outputs: +- a clear contract for what counts as “dashboard ready” +- shared conflict categories for healthy reuse, recoverable ownership, and + explicit remediation +- deterministic stderr wording for broker-daemon and broker-console entrypoints + +Verification: +- every dashboard startup branch ends in either usable dashboard availability or + non-zero failure +- no code path continues with `--web-ui` requested after a port conflict unless + the dashboard is actually reachable + +### Phase 2: Implement shared conflict handling + +Inputs: +- dashboard port probes and listener ownership helpers +- broker PID/socket/version state +- existing `/api/control` and `/api/broker/status` probes + +Outputs: +- reusable conflict-resolution helper(s) for broker startup/orchestration +- explicit reuse path for already-healthy broker-backed dashboards +- fail-fast or restart-assisted path for foreign/stale ownership conflicts + +Verification: +- running broker + healthy dashboard remains a no-op/reuse case +- foreign listeners and broker-without-dashboard states do not continue as + “success” + +### Phase 3: Align diagnostics and finalize validation + +Inputs: +- shared conflict results from startup/orchestration +- doctor and TUI user guidance +- unit and integration tests covering startup/diagnostic flows + +Outputs: +- aligned doctor guidance for port ownership conflicts +- regression tests across broker-daemon, broker-console, and doctor +- validation report with required quality-gate output + +Verification: +- the same runtime state yields the same diagnosis and remediation whether users + encounter it during startup, in TUI, or via `--doctor` +- no existing broker/web-ui/TUI entrypoints regress + +## Acceptance Tests + +- `pytest tests/unit/test_main.py` +- `pytest tests/unit/test_main_tui.py` +- `pytest tests/unit/test_doctor.py` +- `pytest tests/unit/test_tui.py` +- `pytest` +- `ruff check src/` +- `mypy src/` +- `pytest --cov` + +## Decision Points + +- Prefer fail-fast over silent degradation for explicit `--web-ui` requests; a + running broker without the requested dashboard is a broken UX state, not a + successful startup. +- Reuse the existing dashboard probe surfaces instead of inventing a second + ownership model just for startup recovery. +- Keep automatic recovery narrowly scoped to deterministic wrapper-owned cases; + otherwise print one clear remediation path and exit. + +## Notes + +- If implementation exposes common conflict helpers that both `doctor.py` and + `__main__.py` should consume, prefer that refactor now over duplicating + ownership logic again. +- The dedicated-host workflow introduced in `P7-T1` remains the product path; + messaging should continue to steer users toward `--broker-console` or + `--broker-daemon --web-ui`, not ad-hoc manual recovery. +- Review subject name for this task: `dashboard_port_ownership_conflicts`. From b55a2fe25f7ac4bc5097972f77aeda12879aa555 Mon Sep 17 00:00:00 2001 From: Egor Merkushev Date: Sat, 7 Mar 2026 18:30:15 +0300 Subject: [PATCH 4/8] Implement P7-T3: fail fast on broker dashboard conflicts --- AGENTS.md | 2 +- README.md | 4 +- SPECS/INPROGRESS/P7-T3_Validation_Report.md | 100 ++++++++++ .../Documentation.docc/XcodeMCPWrapper.md | 2 +- src/mcpbridge_wrapper/__main__.py | 133 ++++++++++--- tests/unit/test_doctor.py | 17 +- tests/unit/test_main.py | 176 +++++++++++++++++- 7 files changed, 395 insertions(+), 39 deletions(-) create mode 100644 SPECS/INPROGRESS/P7-T3_Validation_Report.md diff --git a/AGENTS.md b/AGENTS.md index 8db4848..140f8b9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -40,7 +40,7 @@ A Python wrapper (`xcodemcpwrapper`) that intercepts responses from `xcrun mcpbr ### Metrics -- **Test Coverage:** 92.19% +- **Test Coverage:** 91.62% - **Performance:** <0.01ms overhead per transformation (0.0023ms avg) - **Memory:** <10MB footprint - **Lines of Code:** ~400 Python + 2000+ lines documentation diff --git a/README.md b/README.md index b6707a4..8387947 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ [![Python 3.9+](https://img.shields.io/badge/python-3.9+-blue.svg)](https://www.python.org/downloads/) [![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://opensource.org/licenses/MIT) -[![Coverage](https://img.shields.io/badge/coverage-90.91%25-brightgreen.svg)](./SPECS/ARCHIVE/P5-T14_Code_Coverage/) +[![Coverage](https://img.shields.io/badge/coverage-91.62%25-brightgreen.svg)](./SPECS/ARCHIVE/P5-T14_Code_Coverage/) [![MCP Registry](https://img.shields.io/badge/MCP%20Registry-io.github.SoundBlaster%2Fxcode--mcpbridge--wrapper-blue)](https://registry.modelcontextprotocol.io) @@ -664,7 +664,7 @@ make test && make lint && make typecheck - **Overhead:** <0.01ms per transformation - **Memory:** <10MB footprint -- **Coverage:** 90.91% test coverage +- **Coverage:** 91.62% test coverage ## License diff --git a/SPECS/INPROGRESS/P7-T3_Validation_Report.md b/SPECS/INPROGRESS/P7-T3_Validation_Report.md new file mode 100644 index 0000000..e4e9cdd --- /dev/null +++ b/SPECS/INPROGRESS/P7-T3_Validation_Report.md @@ -0,0 +1,100 @@ +# P7-T3 Validation Report + +**Task:** P7-T3 — Auto-recover or guide on dashboard port ownership conflicts +**Date:** 2026-03-07 +**Verdict:** PASS + +## Summary + +Implemented the broker-hosted dashboard conflict fix by: + +- removing the silent broker-daemon partial state where `--broker-daemon --web-ui` + could continue without the requested dashboard +- adding shared user-facing remediation helpers in + `src/mcpbridge_wrapper/__main__.py` so startup now resolves occupied-port + states into explicit attach, reset, or restart-assisted guidance +- aligning `--broker-console` conflict messaging with the dedicated-host + workflow already surfaced by `--doctor` +- preserving the safe recovery path via + `mcpbridge-wrapper --broker-console --web-ui-restart` +- extending unit coverage for foreign listeners, running brokers without a + dashboard, already-healthy broker-backed endpoints, and restart-assisted + recovery +- refreshing published coverage references in `README.md`, + `Sources/XcodeMCPWrapper/Documentation.docc/XcodeMCPWrapper.md`, and + `AGENTS.md` to the validated current total + +## Files Validated + +- `src/mcpbridge_wrapper/__main__.py` +- `tests/unit/test_main.py` +- `tests/unit/test_doctor.py` +- `README.md` +- `Sources/XcodeMCPWrapper/Documentation.docc/XcodeMCPWrapper.md` +- `AGENTS.md` + +## Targeted Verification + +```bash +pytest tests/unit/test_main.py tests/unit/test_main_tui.py tests/unit/test_doctor.py tests/unit/test_tui.py +``` + +- Result: `163 passed` + +```bash +pytest tests/unit/test_main.py -k 'broker_console or broker_daemon_webui_port_occupied or broker_lacks_dashboard' +``` + +- Result: `18 passed` + +```bash +python scripts/check_doc_sync.py --all --require-same-commit +``` + +- Result: `PASS` +- Observed outcome: README and DocC coverage references remained in sync after + updating the validated coverage metric + +## Required Quality Gates + +```bash +pytest +``` + +- Result: `887 passed, 5 skipped in 7.98s` + +```bash +python -m ruff check src/ tests/ +``` + +- Result: `All checks passed!` + +```bash +mypy src/ +``` + +- Result: `Success: no issues found in 20 source files` + +```bash +make format-check +``` + +- Result: `55 files already formatted` + +```bash +pytest --cov=src --cov-report=term +``` + +- Result: `887 passed, 5 skipped in 8.97s` +- Coverage: `91.62%` + +## Notes + +- The new startup contract is intentionally fail-fast for explicit `--web-ui` + requests. A running broker without the requested frontend is now treated as a + broken state, not a successful launch. +- Doctor messaging already matched the intended dedicated-host recovery path, so + this task focused on aligning startup/orchestration behavior to that model + instead of inventing a separate recovery surface. +- Remaining warnings are the pre-existing `websockets` / `uvicorn` + deprecations already visible in the repository test suite. diff --git a/Sources/XcodeMCPWrapper/Documentation.docc/XcodeMCPWrapper.md b/Sources/XcodeMCPWrapper/Documentation.docc/XcodeMCPWrapper.md index 34455da..36ae057 100644 --- a/Sources/XcodeMCPWrapper/Documentation.docc/XcodeMCPWrapper.md +++ b/Sources/XcodeMCPWrapper/Documentation.docc/XcodeMCPWrapper.md @@ -497,7 +497,7 @@ Important for multi-agent setups: | Metric | Value | |--------|-------| -| Test Coverage | 90.91% | +| Test Coverage | 91.62% | | Performance Overhead | <0.01ms per transformation | | Memory Footprint | <10MB | diff --git a/src/mcpbridge_wrapper/__main__.py b/src/mcpbridge_wrapper/__main__.py index 08874a3..6ef3d50 100644 --- a/src/mcpbridge_wrapper/__main__.py +++ b/src/mcpbridge_wrapper/__main__.py @@ -576,6 +576,86 @@ def _recent_broker_events_hint(runtime: Any, max_lines: int = 3) -> str: return f" Recent broker events: {compact}" +def _broker_console_command() -> str: + """Return the canonical attach command for the dedicated broker host.""" + return "`mcpbridge-wrapper --broker-console`" + + +def _broker_console_reset_command() -> str: + """Return the canonical full-reset command for the dedicated broker host.""" + return "`mcpbridge-wrapper --broker-stop && mcpbridge-wrapper --broker-console`" + + +def _broker_console_restart_command() -> str: + """Return the canonical restart-assisted recovery command.""" + return "`mcpbridge-wrapper --broker-console --web-ui-restart`" + + +def _format_listener_pid_summary(listener_pids: Set[int]) -> str: + """Render a stable listener-PID summary for user-facing errors.""" + if not listener_pids: + return "unknown listener" + + label = "PID" if len(listener_pids) == 1 else "PIDs" + joined = ", ".join(str(pid) for pid in sorted(listener_pids)) + return f"listener {label} {joined}" + + +def _report_requested_dashboard_unavailable( + *, + runtime: Any, + port: Optional[int], + probe_error: Optional[str], + running_broker_pid: Optional[int], + listener_pids: Set[int], +) -> int: + """Print one explicit remediation path for an unusable requested dashboard.""" + if running_broker_pid is not None: + print( + "Error: Broker daemon is already running " + f"(PID {running_broker_pid}) but no broker-backed dashboard is reachable at " + f"{runtime.base_url}. Restart the dedicated host with " + f"{_broker_console_reset_command()}.", + file=sys.stderr, + ) + elif port is not None and listener_pids: + print( + f"Error: Web UI port {port} is already occupied by " + f"{_format_listener_pid_summary(listener_pids)}. " + f"Stop the existing listener or retry startup with " + f"{_broker_console_restart_command()}.", + file=sys.stderr, + ) + elif port is not None: + print( + f"Error: Web UI port {port} is unavailable and no broker-backed dashboard is " + f"reachable at {runtime.base_url}. Retry startup with " + f"{_broker_console_restart_command()} or choose a different port.", + file=sys.stderr, + ) + else: + print( + f"Error: No broker-backed dashboard is reachable at {runtime.base_url}. " + f"Restart the dedicated host with {_broker_console_reset_command()}.", + file=sys.stderr, + ) + + if probe_error: + print(f"Detail: {probe_error}", file=sys.stderr) + return 1 + + +def _report_existing_broker_dashboard(runtime: Any) -> int: + """Report that the requested dashboard is already served by another broker host.""" + print( + f"Error: Dashboard at {runtime.base_url} is already serving the dedicated broker host. " + f"Use {_broker_console_command()} to attach, or stop the running broker with " + "`mcpbridge-wrapper --broker-stop` before starting a new daemon.", + file=sys.stderr, + ) + return 1 + + def _spawn_broker_console_host( *, web_ui_port: Optional[int], @@ -664,16 +744,13 @@ def _run_broker_console( running_pid = _read_running_broker_pid() if running_pid is not None: - print( - "Error: Broker daemon is already running " - f"(PID {running_pid}) but {runtime.base_url} is not a broker-backed dashboard. " - "Stop it with --broker-stop and restart with --broker-console " - "or --broker-daemon --web-ui.", - file=sys.stderr, + return _report_requested_dashboard_unavailable( + runtime=runtime, + port=None, + probe_error=error, + running_broker_pid=running_pid, + listener_pids=set(), ) - if error: - print(f"Detail: {error}", file=sys.stderr) - return 1 effective_port = _effective_web_ui_port( web_ui_enabled=True, @@ -683,15 +760,13 @@ def _run_broker_console( if effective_port is not None and not web_ui_restart: listener_pids = _find_listener_pids_for_port(effective_port) if listener_pids: - print( - "Error: Dashboard port " - f"{effective_port} is already occupied and is not serving broker-daemon. " - "Stop the existing listener or retry with --web-ui-restart.", - file=sys.stderr, + return _report_requested_dashboard_unavailable( + runtime=runtime, + port=effective_port, + probe_error=error, + running_broker_pid=None, + listener_pids=listener_pids, ) - if error: - print(f"Detail: {error}", file=sys.stderr) - return 1 child_process = _spawn_broker_console_host( web_ui_port=web_ui_port, @@ -983,10 +1058,26 @@ def get_broker_status() -> Optional[Dict[str, Any]]: ) = runtime if not is_port_available(config.host, config.port): - print( - f"Warning: Web UI port {config.port} is already in use. " - "Skipping Web UI startup — broker daemon will run without the dashboard.", - file=sys.stderr, + from mcpbridge_wrapper.tui import build_tui_runtime + + dashboard_runtime = build_tui_runtime( + web_ui_port=config.port, + web_ui_config=web_ui_config, + ) + ready, error = _probe_broker_console_backend(dashboard_runtime) + if ready: + audit.close() + return _report_existing_broker_dashboard(dashboard_runtime) + + running_pid = _read_running_broker_pid() + listener_pids = _find_listener_pids_for_port(config.port) + audit.close() + return _report_requested_dashboard_unavailable( + runtime=dashboard_runtime, + port=config.port, + probe_error=error, + running_broker_pid=running_pid, + listener_pids=listener_pids, ) else: diff --git a/tests/unit/test_doctor.py b/tests/unit/test_doctor.py index 77a3e74..6ae85cb 100644 --- a/tests/unit/test_doctor.py +++ b/tests/unit/test_doctor.py @@ -307,8 +307,11 @@ def test_classify_broker_running_without_dashboard(self) -> None: assert report.ok is False assert report.code == "broker-without-dashboard" assert "no broker-backed dashboard" in report.summary.lower() - assert "--broker-stop" in report.next_action - assert "--broker-console" in report.next_action + assert ( + report.next_action + == "Restart the dedicated host with `mcpbridge-wrapper --broker-stop && " + "mcpbridge-wrapper --broker-console`." + ) def test_classify_wrong_service_on_dashboard_port(self) -> None: dashboard = _dashboard( @@ -337,7 +340,10 @@ def test_classify_wrong_service_on_dashboard_port(self) -> None: assert report.ok is False assert report.code == "wrong-service" assert "not the dedicated broker host" in report.summary.lower() - assert "--web-ui-restart" in report.next_action + assert ( + report.next_action == "Stop the existing listener or retry startup with " + "`mcpbridge-wrapper --broker-console --web-ui-restart`." + ) def test_classify_broker_daemon_with_unavailable_runtime_status(self) -> None: dashboard = _dashboard( @@ -391,7 +397,10 @@ def test_classify_port_occupied_before_broker_startup(self) -> None: assert report.ok is False assert report.code == "port-occupied" assert "occupied" in report.summary.lower() - assert "--web-ui-restart" in report.next_action + assert ( + report.next_action == "Free the port or retry startup with " + "`mcpbridge-wrapper --broker-console --web-ui-restart`." + ) def test_classify_broker_not_running_without_any_endpoint(self) -> None: report = classify_doctor_report( diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index a225c92..79f0982 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -1759,7 +1759,10 @@ def test_run_broker_console_rejects_stale_running_broker(self, capsys): ) assert result == 1 - assert "Broker daemon is already running (PID 4242)" in capsys.readouterr().err + err = capsys.readouterr().err + assert "Broker daemon is already running (PID 4242)" in err + assert "--broker-stop" in err + assert "--broker-console" in err spawn_host.assert_not_called() def test_run_broker_console_rejects_foreign_dashboard_listener(self, capsys): @@ -1789,7 +1792,9 @@ def test_run_broker_console_rejects_foreign_dashboard_listener(self, capsys): ) assert result == 1 - assert "Dashboard port 8080 is already occupied" in capsys.readouterr().err + err = capsys.readouterr().err + assert "Web UI port 8080 is already occupied" in err + assert "--broker-console --web-ui-restart" in err spawn_host.assert_not_called() def test_run_broker_console_returns_1_when_dashboard_never_becomes_ready(self, capsys): @@ -1829,6 +1834,52 @@ def test_run_broker_console_returns_1_when_dashboard_never_becomes_ready(self, c assert "Timed out waiting for dashboard" in capsys.readouterr().err run_tui.assert_not_called() + def test_run_broker_console_uses_restart_mode_to_recover_occupied_port(self): + from mcpbridge_wrapper.__main__ import _run_broker_console + + runtime = SimpleNamespace(base_url="http://127.0.0.1:8080", log_path="/tmp/broker.log") + process = MagicMock() + with patch( + "mcpbridge_wrapper.tui.build_tui_runtime", + return_value=runtime, + ), patch( + "mcpbridge_wrapper.__main__._probe_broker_console_backend", + return_value=(False, "GET /api/broker/status failed: Not Found"), + ), patch( + "mcpbridge_wrapper.__main__._read_running_broker_pid", + return_value=None, + ), patch( + "mcpbridge_wrapper.__main__._effective_web_ui_port", + return_value=8080, + ), patch( + "mcpbridge_wrapper.__main__._find_listener_pids_for_port", + return_value={999}, + ) as find_listeners, patch( + "mcpbridge_wrapper.__main__._spawn_broker_console_host", + return_value=process, + ) as spawn_host, patch( + "mcpbridge_wrapper.__main__._wait_for_broker_console_backend", + return_value=None, + ) as wait_ready, patch( + "mcpbridge_wrapper.tui.run_tui", + return_value=0, + ) as run_tui: + result = _run_broker_console( + web_ui_port=8080, + web_ui_config="/tmp/webui.json", + web_ui_restart=True, + ) + + assert result == 0 + find_listeners.assert_not_called() + spawn_host.assert_called_once_with( + web_ui_port=8080, + web_ui_config="/tmp/webui.json", + web_ui_restart=True, + ) + wait_ready.assert_called_once_with(runtime, child_process=process) + run_tui.assert_called_once_with(runtime) + def test_run_broker_console_returns_0_on_keyboard_interrupt(self): from mcpbridge_wrapper.__main__ import _run_broker_console @@ -2068,10 +2119,8 @@ def test_main_broker_daemon_webui_runtime_failure_returns_1(self): assert result == 1 mock_daemon_cls.assert_not_called() - def test_main_broker_daemon_webui_port_occupied_skips_dashboard_thread(self): + def test_main_broker_daemon_webui_port_occupied_fails_fast_with_restart_guidance(self, capsys): broker_cfg = MagicMock() - daemon = MagicMock() - transport = MagicMock() webui_config = MagicMock(spec=WebUIConfig) webui_config.host = "127.0.0.1" webui_config.port = 8080 @@ -2094,17 +2143,124 @@ def test_main_broker_daemon_webui_port_occupied_skips_dashboard_thread(self): run_server, run_server_in_thread, ), + ), patch( + "mcpbridge_wrapper.tui.build_tui_runtime", + return_value=SimpleNamespace(base_url="http://127.0.0.1:8080"), + ), patch( + "mcpbridge_wrapper.__main__._probe_broker_console_backend", + return_value=(False, "GET /api/broker/status failed: Not Found"), + ), patch( + "mcpbridge_wrapper.__main__._read_running_broker_pid", + return_value=None, + ), patch( + "mcpbridge_wrapper.__main__._find_listener_pids_for_port", + return_value={999}, ), patch("mcpbridge_wrapper.broker.types.BrokerConfig") as mock_cfg_cls, patch( - "mcpbridge_wrapper.broker.daemon.BrokerDaemon", return_value=daemon + "mcpbridge_wrapper.broker.daemon.BrokerDaemon" + ) as mock_daemon_cls: + mock_cfg_cls.default.return_value = broker_cfg + result = main() + + assert result == 1 + run_server_in_thread.assert_not_called() + err = capsys.readouterr().err + assert "Web UI port 8080 is already occupied" in err + assert "--web-ui-restart" in err + assert "GET /api/broker/status failed: Not Found" in err + mock_daemon_cls.assert_not_called() + audit.close.assert_called_once_with() + + def test_main_broker_daemon_webui_fails_fast_when_broker_lacks_dashboard(self, capsys): + broker_cfg = MagicMock() + webui_config = MagicMock(spec=WebUIConfig) + webui_config.host = "127.0.0.1" + webui_config.port = 8080 + metrics = MagicMock() + audit = MagicMock() + is_port_available = MagicMock(return_value=False) + run_server = MagicMock() + run_server_in_thread = MagicMock() + + with patch( + "mcpbridge_wrapper.__main__.sys.argv", + ["mcpbridge-wrapper", "--broker-daemon", "--web-ui"], ), patch( - "mcpbridge_wrapper.broker.transport.UnixSocketServer", - return_value=transport, - ), patch("asyncio.run"): + "mcpbridge_wrapper.__main__._prepare_webui_runtime", + return_value=( + webui_config, + metrics, + audit, + is_port_available, + run_server, + run_server_in_thread, + ), + ), patch( + "mcpbridge_wrapper.tui.build_tui_runtime", + return_value=SimpleNamespace(base_url="http://127.0.0.1:8080"), + ), patch( + "mcpbridge_wrapper.__main__._probe_broker_console_backend", + return_value=(False, "Cannot reach http://127.0.0.1:8080: refused"), + ), patch( + "mcpbridge_wrapper.__main__._read_running_broker_pid", + return_value=4242, + ), patch("mcpbridge_wrapper.broker.types.BrokerConfig") as mock_cfg_cls, patch( + "mcpbridge_wrapper.broker.daemon.BrokerDaemon" + ) as mock_daemon_cls: mock_cfg_cls.default.return_value = broker_cfg result = main() - assert result == 0 + assert result == 1 + err = capsys.readouterr().err + assert "Broker daemon is already running (PID 4242)" in err + assert "--broker-stop" in err + assert "--broker-console" in err + assert "Cannot reach http://127.0.0.1:8080: refused" in err run_server_in_thread.assert_not_called() + mock_daemon_cls.assert_not_called() + audit.close.assert_called_once_with() + + def test_main_broker_daemon_webui_reuses_existing_backend_as_explicit_error(self, capsys): + broker_cfg = MagicMock() + webui_config = MagicMock(spec=WebUIConfig) + webui_config.host = "127.0.0.1" + webui_config.port = 8080 + metrics = MagicMock() + audit = MagicMock() + is_port_available = MagicMock(return_value=False) + run_server = MagicMock() + run_server_in_thread = MagicMock() + + with patch( + "mcpbridge_wrapper.__main__.sys.argv", + ["mcpbridge-wrapper", "--broker-daemon", "--web-ui"], + ), patch( + "mcpbridge_wrapper.__main__._prepare_webui_runtime", + return_value=( + webui_config, + metrics, + audit, + is_port_available, + run_server, + run_server_in_thread, + ), + ), patch( + "mcpbridge_wrapper.tui.build_tui_runtime", + return_value=SimpleNamespace(base_url="http://127.0.0.1:8080"), + ), patch( + "mcpbridge_wrapper.__main__._probe_broker_console_backend", + return_value=(True, None), + ), patch("mcpbridge_wrapper.broker.types.BrokerConfig") as mock_cfg_cls, patch( + "mcpbridge_wrapper.broker.daemon.BrokerDaemon" + ) as mock_daemon_cls: + mock_cfg_cls.default.return_value = broker_cfg + result = main() + + assert result == 1 + err = capsys.readouterr().err + assert "already serving the dedicated broker host" in err + assert "--broker-console" in err + run_server_in_thread.assert_not_called() + mock_daemon_cls.assert_not_called() audit.close.assert_called_once_with() From 4c41435fae4d53705105613f407fbe2a93bffb7a Mon Sep 17 00:00:00 2001 From: Egor Merkushev Date: Sat, 7 Mar 2026 18:32:04 +0300 Subject: [PATCH 5/8] Archive task P7-T3: Auto-recover or guide on dashboard port ownership conflicts (PASS) --- SPECS/ARCHIVE/INDEX.md | 4 ++- ...e_on_dashboard_port_ownership_conflicts.md | 4 +++ .../P7-T3_Validation_Report.md | 0 SPECS/INPROGRESS/next.md | 25 ++++++++++--------- SPECS/Workplan.md | 17 +++++++------ 5 files changed, 29 insertions(+), 21 deletions(-) rename SPECS/{INPROGRESS => ARCHIVE/P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts}/P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts.md (99%) rename SPECS/{INPROGRESS => ARCHIVE/P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts}/P7-T3_Validation_Report.md (100%) diff --git a/SPECS/ARCHIVE/INDEX.md b/SPECS/ARCHIVE/INDEX.md index 379237c..9b413f6 100644 --- a/SPECS/ARCHIVE/INDEX.md +++ b/SPECS/ARCHIVE/INDEX.md @@ -1,11 +1,12 @@ # mcpbridge-wrapper Tasks Archive -**Last Updated:** 2026-03-07 (P7-T2 review archived) +**Last Updated:** 2026-03-07 (P7-T3 archived) ## Archived Tasks | Task ID | Folder | Archived | Verdict | |---------|--------|----------|---------| +| P7-T3 | [P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts/](P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts/) | 2026-03-07 | PASS | | P7-T2 | [P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics/](P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics/) | 2026-03-07 | PASS | | P7-T1 | [P7-T1_Add_one-command_broker_host_startup_with_attached_frontend/](P7-T1_Add_one-command_broker_host_startup_with_attached_frontend/) | 2026-03-07 | PASS | | P6-T3 | [P6-T3_Document_the_explicit_dedicated-host_frontend_workflow/](P6-T3_Document_the_explicit_dedicated-host_frontend_workflow/) | 2026-03-07 | PASS | @@ -609,3 +610,4 @@ | 2026-03-07 | P6-T2 | Archived REVIEW_broker_terminal_frontend report | | 2026-03-07 | P7-T2 | Archived Implement_a_broker_doctor_command_for_cross-black-box_diagnostics (PASS) | | 2026-03-07 | P7-T2 | Archived REVIEW_broker_doctor_diagnostics report | +| 2026-03-07 | P7-T3 | Archived Auto-recover_or_guide_on_dashboard_port_ownership_conflicts (PASS) | diff --git a/SPECS/INPROGRESS/P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts.md b/SPECS/ARCHIVE/P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts/P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts.md similarity index 99% rename from SPECS/INPROGRESS/P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts.md rename to SPECS/ARCHIVE/P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts/P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts.md index 0f644ce..5355113 100644 --- a/SPECS/INPROGRESS/P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts.md +++ b/SPECS/ARCHIVE/P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts/P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts.md @@ -147,3 +147,7 @@ Verification: messaging should continue to steer users toward `--broker-console` or `--broker-daemon --web-ui`, not ad-hoc manual recovery. - Review subject name for this task: `dashboard_port_ownership_conflicts`. + +--- +**Archived:** 2026-03-07 +**Verdict:** PASS diff --git a/SPECS/INPROGRESS/P7-T3_Validation_Report.md b/SPECS/ARCHIVE/P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts/P7-T3_Validation_Report.md similarity index 100% rename from SPECS/INPROGRESS/P7-T3_Validation_Report.md rename to SPECS/ARCHIVE/P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts/P7-T3_Validation_Report.md diff --git a/SPECS/INPROGRESS/next.md b/SPECS/INPROGRESS/next.md index 062aa83..66d9757 100644 --- a/SPECS/INPROGRESS/next.md +++ b/SPECS/INPROGRESS/next.md @@ -1,24 +1,25 @@ -# Next Task: P7-T3 — Auto-recover or guide on dashboard port ownership conflicts +# Next Task: P7-T4 — Add direct local-status fallback for TUI when dashboard API is unavailable -**Priority:** P0 +**Priority:** P1 **Phase:** Phase 7: Broker UX and Diagnostics -**Effort:** 4-6 hours -**Dependencies:** P6-T1, P7-T2 -**Status:** Selected +**Effort:** 4-5 hours +**Dependencies:** P6-T2 +**Status:** Ready ## Description -Improve the broker-hosted dashboard startup path so users do not get stranded -when the desired Web UI port is occupied by a stale or unrelated process. -Prefer deterministic recovery or one explicit remediation path over the current -partial state where the broker can stay alive without the dashboard/frontend -required by the recommended UX flow. +Reduce TUI dependence on the dashboard HTTP API by letting it fall back to the +best available local broker state when the dashboard endpoint is unavailable. +Users should still be able to tell whether the broker is alive, whether the +frontend/control plane is degraded, and which restart/recovery step to take +next without leaving the TUI. ## Recently Archived +- `2026-03-07` — `P7-T3` archived with verdict `PASS` - `2026-03-07` — `P7-T2` archived with verdict `PASS` ## Next Step -Create the `P7-T3` PRD in `SPECS/INPROGRESS/`, then implement and validate the -dashboard port-conflict recovery path. +Create the `P7-T4` PRD in `SPECS/INPROGRESS/`, then implement and validate the +local broker-status fallback path for TUI. diff --git a/SPECS/Workplan.md b/SPECS/Workplan.md index 1e7cbd0..b74f5ed 100644 --- a/SPECS/Workplan.md +++ b/SPECS/Workplan.md @@ -512,19 +512,20 @@ Add new tasks using the canonical template in [TASK_TEMPLATE.md](TASK_TEMPLATE.m - [x] The diagnostics distinguish between “broker alive but no dashboard”, “dashboard alive but wrong service”, “port already occupied”, and “broker not running” - [x] Output is user-facing and actionable without requiring users to manually run `lsof`, `curl`, or inspect raw log files first -#### ⬜️ P7-T3: Auto-recover or guide on dashboard port ownership conflicts -- **Description:** Improve the broker-hosted dashboard startup path so users do not get stranded when the desired Web UI port is occupied by a stale or unrelated process. Prefer deterministic recovery or explicit guided remediation over the current “skip dashboard startup and keep broker alive” behavior, which leaves TUI and browser UX in a confusing partial state. +#### ✅ P7-T3: Auto-recover or guide on dashboard port ownership conflicts +- **Status:** ✅ Completed (2026-03-07) +- **Description:** Remove the silent partial state where `--broker-daemon --web-ui` can keep a broker alive without the requested dashboard. Startup now fails fast with one explicit remediation path or points users at the already-healthy broker-backed frontend, while `--broker-console --web-ui-restart` remains the safe recovery path for occupied ports. - **Priority:** P0 - **Dependencies:** P6-T1, P7-T2 - **Parallelizable:** no - **Outputs/Artifacts:** - - broker startup logic and/or orchestration flow updated to detect occupied dashboard ports and choose a clearer recovery path - - improved stderr/TUI/doctor messaging for port ownership conflicts - - tests covering stale dashboard owner, live foreign process on the port, and successful recovery paths + - broker startup/orchestration flow updated to fail fast on unusable dashboard ports instead of silently degrading + - improved stderr guidance aligned with the dedicated-host `doctor` / broker-console workflow + - tests covering foreign listeners, running brokers without dashboards, healthy existing dashboards, and restart-assisted recovery - **Acceptance Criteria:** - - [ ] Users are not left with a running broker that silently lacks the dashboard/frontend required by the recommended UX path - - [ ] Port conflicts result in either automatic safe recovery or one explicit remediation path with exact commands or next steps - - [ ] TUI and diagnostics clearly explain the conflict source and whether the current runtime is usable + - [x] Users are not left with a running broker that silently lacks the dashboard/frontend required by the recommended UX path + - [x] Port conflicts result in either automatic safe recovery or one explicit remediation path with exact commands or next steps + - [x] TUI and diagnostics clearly explain the conflict source and whether the current runtime is usable #### ⬜️ P7-T4: Add direct local-status fallback for TUI when dashboard API is unavailable - **Description:** Reduce TUI dependence on the Web UI API by letting it fall back to local broker state when the dashboard endpoint is unavailable. The TUI should still provide useful diagnostics from PID/socket/version files and any directly accessible broker status sources, while clearly indicating that live dashboard-backed controls are unavailable. From f4c4d58079341f085c3d5cb18843c7fa5df97a69 Mon Sep 17 00:00:00 2001 From: Egor Merkushev Date: Sat, 7 Mar 2026 18:33:46 +0300 Subject: [PATCH 6/8] Review P7-T3: dashboard port ownership conflicts --- ...VIEW_dashboard_port_ownership_conflicts.md | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 SPECS/INPROGRESS/REVIEW_dashboard_port_ownership_conflicts.md diff --git a/SPECS/INPROGRESS/REVIEW_dashboard_port_ownership_conflicts.md b/SPECS/INPROGRESS/REVIEW_dashboard_port_ownership_conflicts.md new file mode 100644 index 0000000..907777c --- /dev/null +++ b/SPECS/INPROGRESS/REVIEW_dashboard_port_ownership_conflicts.md @@ -0,0 +1,55 @@ +## REVIEW REPORT — dashboard_port_ownership_conflicts + +**Scope:** origin/main..HEAD +**Files:** 11 + +### Summary Verdict +- [ ] Approve +- [x] Approve with comments +- [ ] Request changes +- [ ] Block + +### Critical Issues + +None. + +### Secondary Issues + +- [Medium] `src/mcpbridge_wrapper/__main__.py:604-640`, + `src/mcpbridge_wrapper/__main__.py:1072-1080`: + `_report_requested_dashboard_unavailable()` prioritizes + `running_broker_pid` over `listener_pids`. In the mixed state where a broker + PID is still live but the requested dashboard port is actually occupied by an + unrelated listener, both `--broker-console` and `--broker-daemon --web-ui` + will tell users to reset the broker first and omit the foreign port owner. + That remediation can loop back into the same conflict because the real + blocker, the non-broker listener on the port, was never surfaced. The + conflict classifier should prefer observable foreign port ownership or name + both blockers in one explicit recovery path. + +### Architectural Notes + +- The task correctly removes the most confusing partial state: explicit + dashboard startup no longer silently degrades into “broker alive, dashboard + absent”. +- Keeping `--broker-console --web-ui-restart` as the safe recovery path is the + right product choice; the remaining issue is about mixed-state prioritization, + not the core startup contract. +- Coverage-reference updates were kept in lockstep across README and the DocC + mirror, which avoids a repeat of the earlier documentation drift problem. + +### Tests + +- `pytest tests/unit/test_main.py tests/unit/test_main_tui.py tests/unit/test_doctor.py tests/unit/test_tui.py` passed (`163 passed`) +- `pytest` passed (`887 passed, 5 skipped`) +- `python -m ruff check src/ tests/` passed +- `mypy src/` passed +- `make format-check` passed +- `python scripts/check_doc_sync.py --all --require-same-commit` passed +- `pytest --cov=src --cov-report=term` passed with `91.62%` coverage + +### Next Steps + +- Add a focused follow-up task to improve mixed-state conflict prioritization + when a live broker PID and a foreign dashboard-port listener are both + observable. From eb0958c41d9e7125e774d675c62f12dc53fffd6c Mon Sep 17 00:00:00 2001 From: Egor Merkushev Date: Sat, 7 Mar 2026 18:34:18 +0300 Subject: [PATCH 7/8] Follow-up P7-T3: mixed-state dashboard conflict prioritization --- SPECS/Workplan.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/SPECS/Workplan.md b/SPECS/Workplan.md index b74f5ed..b62f278 100644 --- a/SPECS/Workplan.md +++ b/SPECS/Workplan.md @@ -527,6 +527,20 @@ Add new tasks using the canonical template in [TASK_TEMPLATE.md](TASK_TEMPLATE.m - [x] Port conflicts result in either automatic safe recovery or one explicit remediation path with exact commands or next steps - [x] TUI and diagnostics clearly explain the conflict source and whether the current runtime is usable +#### ⬜️ FU-P7-T3-1: Prioritize foreign port-owner guidance in mixed broker/dashboard conflicts +- **Description:** When startup sees both a live broker PID and a non-broker listener on the requested dashboard port, current remediation prioritizes broker reset guidance and can hide the actual foreign port owner. Update startup and diagnostics conflict ordering so users see the real blocker or one combined recovery path instead of being sent into a reset loop. +- **Priority:** P1 +- **Dependencies:** P7-T3 +- **Parallelizable:** yes +- **Outputs/Artifacts:** + - mixed-state broker/dashboard conflict classifier or message ordering updates + - startup and diagnostics tests covering live broker PID plus foreign dashboard-port listener + - any wording changes needed so recovery stays explicit and single-path +- **Acceptance Criteria:** + - [ ] `--broker-console` and `--broker-daemon --web-ui` surface the foreign dashboard-port owner or both blockers when a live broker PID and foreign listener coexist + - [ ] `--doctor` does not hide the foreign listener behind a generic broker-without-dashboard diagnosis in the same mixed state + - [ ] Regression tests cover the mixed-state conflict and prevent reordering back to broker-only guidance + #### ⬜️ P7-T4: Add direct local-status fallback for TUI when dashboard API is unavailable - **Description:** Reduce TUI dependence on the Web UI API by letting it fall back to local broker state when the dashboard endpoint is unavailable. The TUI should still provide useful diagnostics from PID/socket/version files and any directly accessible broker status sources, while clearly indicating that live dashboard-backed controls are unavailable. - **Priority:** P1 From 8e84c287d28ed173f213c9195ff1ca062ff338a1 Mon Sep 17 00:00:00 2001 From: Egor Merkushev Date: Sat, 7 Mar 2026 18:34:48 +0300 Subject: [PATCH 8/8] Archive REVIEW_dashboard_port_ownership_conflicts report --- SPECS/ARCHIVE/INDEX.md | 4 +++- .../_Historical}/REVIEW_dashboard_port_ownership_conflicts.md | 0 2 files changed, 3 insertions(+), 1 deletion(-) rename SPECS/{INPROGRESS => ARCHIVE/_Historical}/REVIEW_dashboard_port_ownership_conflicts.md (100%) diff --git a/SPECS/ARCHIVE/INDEX.md b/SPECS/ARCHIVE/INDEX.md index 9b413f6..5ba163c 100644 --- a/SPECS/ARCHIVE/INDEX.md +++ b/SPECS/ARCHIVE/INDEX.md @@ -1,6 +1,6 @@ # mcpbridge-wrapper Tasks Archive -**Last Updated:** 2026-03-07 (P7-T3 archived) +**Last Updated:** 2026-03-07 (P7-T3 review archived) ## Archived Tasks @@ -199,6 +199,7 @@ | File | Description | |------|-------------| +| [REVIEW_dashboard_port_ownership_conflicts.md](_Historical/REVIEW_dashboard_port_ownership_conflicts.md) | Review report for P7-T3 | | [REVIEW_broker_doctor_diagnostics.md](_Historical/REVIEW_broker_doctor_diagnostics.md) | Review report for P7-T2 | | [REVIEW_broker_console_startup.md](_Historical/REVIEW_broker_console_startup.md) | Review report for P7-T1 | | [REVIEW_dedicated_host_frontend_docs.md](_Historical/REVIEW_dedicated_host_frontend_docs.md) | Review report for P6-T3 | @@ -611,3 +612,4 @@ | 2026-03-07 | P7-T2 | Archived Implement_a_broker_doctor_command_for_cross-black-box_diagnostics (PASS) | | 2026-03-07 | P7-T2 | Archived REVIEW_broker_doctor_diagnostics report | | 2026-03-07 | P7-T3 | Archived Auto-recover_or_guide_on_dashboard_port_ownership_conflicts (PASS) | +| 2026-03-07 | P7-T3 | Archived REVIEW_dashboard_port_ownership_conflicts report | diff --git a/SPECS/INPROGRESS/REVIEW_dashboard_port_ownership_conflicts.md b/SPECS/ARCHIVE/_Historical/REVIEW_dashboard_port_ownership_conflicts.md similarity index 100% rename from SPECS/INPROGRESS/REVIEW_dashboard_port_ownership_conflicts.md rename to SPECS/ARCHIVE/_Historical/REVIEW_dashboard_port_ownership_conflicts.md