From c97dd1eb117376d9efedc2717f01281a792f2f5d Mon Sep 17 00:00:00 2001 From: Egor Merkushev Date: Sat, 7 Mar 2026 17:49:44 +0300 Subject: [PATCH 1/7] Branch for P7-T2: implement broker doctor diagnostics From b7040d9d1cb72332ff2b2e51e54abc5550ce9c54 Mon Sep 17 00:00:00 2001 From: Egor Merkushev Date: Sat, 7 Mar 2026 17:51:47 +0300 Subject: [PATCH 2/7] Select task P7-T2: Implement a broker doctor command for cross-black-box diagnostics --- SPECS/INPROGRESS/next.md | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/SPECS/INPROGRESS/next.md b/SPECS/INPROGRESS/next.md index 4983bca..901c90b 100644 --- a/SPECS/INPROGRESS/next.md +++ b/SPECS/INPROGRESS/next.md @@ -2,8 +2,9 @@ **Priority:** P0 **Phase:** Phase 7: Broker UX and Diagnostics +**Effort:** 4-6 hours **Dependencies:** P6-T1 -**Status:** Ready after `P7-T1` CI clears +**Status:** Selected ## Description @@ -13,11 +14,6 @@ ownership, and common failure modes such as stale ports, missing dashboard, or wrong endpoint. The output should help users debug without needing to infer the internal broker architecture first. -## Recently Archived - -- `2026-03-07` — `P7-T1` archived with verdict `PASS` - ## Next Step -Wait for the `P7-T1` pull request to clear CI, then run FLOW again to select -and plan `P7-T2`. +Run the PLAN command to produce the implementation-ready PRD for `P7-T2`. From 9d2a0ebcc0259ad0415c3eb4bae3bfcdc8d438c6 Mon Sep 17 00:00:00 2001 From: Egor Merkushev Date: Sat, 7 Mar 2026 17:51:52 +0300 Subject: [PATCH 3/7] Plan task P7-T2: Implement a broker doctor command for cross-black-box diagnostics --- ...command_for_cross-black-box_diagnostics.md | 141 ++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 SPECS/INPROGRESS/P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics.md diff --git a/SPECS/INPROGRESS/P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics.md b/SPECS/INPROGRESS/P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics.md new file mode 100644 index 0000000..6731427 --- /dev/null +++ b/SPECS/INPROGRESS/P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics.md @@ -0,0 +1,141 @@ +# P7-T2 — Implement a broker doctor command for cross-black-box diagnostics + +## Objective Summary + +The new broker-console flow reduced startup friction, but users still hit +multi-layer failures that are hard to interpret: Python environment mismatch, +stale broker files, a live broker without dashboard, a dashboard port owned by +the wrong process, or a reachable dashboard that is not the dedicated +broker-hosted runtime. `P7-T2` should provide one explicit doctor command that +explains those states in user-facing language and points to the single next +action instead of sending users to `lsof`, `curl`, or raw logs. + +The doctor command should build on the status surfaces already introduced for +the TUI and broker daemon rather than inventing a parallel diagnostics stack. +It should classify the visible runtime into actionable buckets, report the +evidence behind that classification, and expose enough detail to help users +understand whether the broken box is Xcode, the IDE-side MCP config, or the +local Python/broker host. + +## Deliverables + +- Add a dedicated user-facing diagnostics mode such as `--doctor` wired through + `src/mcpbridge_wrapper/__main__.py`. +- Introduce a reusable diagnostics module that inspects: + - package/runtime identity and local executable context + - broker PID/socket/version files and live process state + - dashboard endpoint health, ownership, and broker-backed status + - broker runtime payload when the endpoint is broker-backed +- Print concise diagnosis output with one primary status, supporting evidence, + and the most likely next action. +- Add tests for healthy, broker-without-dashboard, foreign-dashboard-owner, + and stale-runtime scenarios. + +## Success Criteria + +- Users can run one command and immediately learn whether broker mode is healthy + and, if not, what exact remediation step to take next. +- The doctor output distinguishes at least these cases: + - broker not running + - broker running without dashboard + - dashboard alive but not broker-backed + - port occupied by another listener before broker startup + - broker/dashboard version or runtime mismatch when observable +- The implementation reuses existing runtime probes where possible so TUI and + future UX tasks can share the same diagnosis logic. + +## Test-First Plan + +1. Add CLI tests that pin `--doctor` flag parsing, incompatibilities with + bridge-only arguments, and exit-code behavior for healthy vs degraded + diagnoses. +2. Add diagnostics-unit tests that feed mocked local files, PID checks, port + listeners, and HTTP probe results into the doctor classifier for the core + runtime buckets. +3. Add rendering tests that ensure output stays actionable and names the exact + recommended next action for each major failure class. +4. Only after the behavior is pinned, implement the diagnostics module and wire + it into `main()`. +5. Run required quality gates: `pytest`, `ruff check src/`, `mypy src/`, and + `pytest --cov`. + +## Execution Plan + +### Phase 1: Doctor contract and classification model + +Inputs: +- `src/mcpbridge_wrapper/__main__.py` +- existing broker/TUI helpers for local PID and dashboard probes + +Outputs: +- a stable command shape for doctor mode +- diagnosis categories and exit-code policy +- reusable data structures for findings, summary, and next-action guidance + +Verification: +- healthy and degraded states map to deterministic summary strings +- the model can represent both local-file evidence and HTTP-probe evidence + +### Phase 2: Local and remote probe implementation + +Inputs: +- `BrokerConfig.default()` state files +- local PID/port ownership probes +- dashboard `/api/control`, `/api/health`, and `/api/broker/status` + +Outputs: +- consolidated probe helpers for local broker state +- dashboard ownership checks that distinguish “wrong service” from + “broker-backed but degraded” +- Xcode/upstream status surfaced when the broker-backed dashboard is reachable + +Verification: +- no-shell-debugging scenarios from the workplan can be distinguished from one + another by the probe results alone +- endpoint failures include enough detail for direct user remediation + +### Phase 3: User-facing rendering, CLI wiring, and regression tests + +Inputs: +- doctor probe/classification results +- `__main__.py` mode validation rules +- existing unit test suites for main and TUI flows + +Outputs: +- final text output for `--doctor` +- CLI integration in `main()` +- regression tests covering the core scenarios and exit codes +- validation report with required quality-gate results + +Verification: +- output stays concise but actionable in every failure mode +- doctor mode does not regress existing broker/TUI/web-ui entrypoints + +## Acceptance Tests + +- `pytest tests/unit/test_main.py` +- `pytest tests/unit/test_main_tui.py` +- `pytest tests/unit/test_tui.py` +- `pytest tests/unit/test_doctor.py` +- `pytest` +- `ruff check src/` +- `mypy src/` +- `pytest --cov` + +## Decision Points + +- Prefer a flag-based entrypoint (`--doctor`) to match the current CLI style and + avoid introducing a separate subcommand parser mid-stream. +- Treat doctor as a shared diagnostics surface, not as a TUI-only helper, so + later tasks can reuse the same findings in TUI fallback and docs. +- Keep the primary output human-readable first; structured export can remain a + future enhancement if needed. + +## Notes + +- Minimize duplication with `tui.py` and current broker-console probes; if local + status helpers must move into a shared module, do that now instead of growing + another copy. +- Any new remediation text should align with the recommended dedicated-host + workflow introduced in `P7-T1`. +- Review subject name for this task: `broker_doctor_diagnostics`. From 48a77bf945a717cb5486b3d077a2ad785563281b Mon Sep 17 00:00:00 2001 From: Egor Merkushev Date: Sat, 7 Mar 2026 18:03:44 +0300 Subject: [PATCH 4/7] Implement P7-T2: add broker doctor diagnostics --- SPECS/INPROGRESS/P7-T2_Validation_Report.md | 94 ++++ src/mcpbridge_wrapper/__main__.py | 49 +- src/mcpbridge_wrapper/doctor.py | 580 ++++++++++++++++++++ tests/unit/test_doctor.py | 558 +++++++++++++++++++ tests/unit/test_main_doctor.py | 80 +++ 5 files changed, 1360 insertions(+), 1 deletion(-) create mode 100644 SPECS/INPROGRESS/P7-T2_Validation_Report.md create mode 100644 src/mcpbridge_wrapper/doctor.py create mode 100644 tests/unit/test_doctor.py create mode 100644 tests/unit/test_main_doctor.py diff --git a/SPECS/INPROGRESS/P7-T2_Validation_Report.md b/SPECS/INPROGRESS/P7-T2_Validation_Report.md new file mode 100644 index 0000000..ea20d2a --- /dev/null +++ b/SPECS/INPROGRESS/P7-T2_Validation_Report.md @@ -0,0 +1,94 @@ +# P7-T2 Validation Report + +**Task:** P7-T2 — Implement a broker doctor command for cross-black-box diagnostics +**Date:** 2026-03-07 +**Verdict:** PASS + +## Summary + +Implemented a user-facing broker diagnostics command by: + +- adding `--doctor` as a dedicated CLI mode in + `src/mcpbridge_wrapper/__main__.py` +- introducing `src/mcpbridge_wrapper/doctor.py` to collect and classify Python + runtime identity, local broker files/processes, dashboard ownership, and + broker-backed runtime state +- distinguishing actionable diagnosis buckets such as healthy runtime, + version mismatch, stale local state, broker without dashboard, wrong + dashboard service, occupied dashboard port, and degraded broker-backed state +- rendering one concise user-facing report with summary, next action, and + supporting evidence instead of requiring manual `lsof` / `curl` debugging +- adding focused unit coverage for helper probes, classification branches, + CLI wiring, and output rendering + +## Files Validated + +- `src/mcpbridge_wrapper/__main__.py` +- `src/mcpbridge_wrapper/doctor.py` +- `tests/unit/test_doctor.py` +- `tests/unit/test_main_doctor.py` + +## Targeted Verification + +```bash +pytest tests/unit/test_doctor.py tests/unit/test_main_doctor.py +``` + +- Result: `30 passed` + +```bash +pytest tests/unit/test_doctor.py tests/unit/test_main_doctor.py --cov=mcpbridge_wrapper.doctor --cov-report=term-missing --no-cov-on-fail +``` + +- Result: `30 passed` +- Doctor module coverage: `90.68%` + +```bash +python -m mcpbridge_wrapper --doctor +``` + +- Result: command executed successfully in the local dedicated-host setup +- Observed summary: `Status: OK` +- Observed diagnosis: broker daemon and broker-backed dashboard were reported healthy + +## Required Quality Gates + +```bash +pytest +``` + +- Result: `884 passed, 5 skipped in 7.98s` + +```bash +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: `884 passed, 5 skipped in 8.90s` +- Coverage: `91.72%` + +## Notes + +- The new doctor output is human-readable first and optimized for the + dedicated-host broker workflow introduced in `P7-T1`. +- The CLI smoke test used the current local runtime and confirmed that doctor + can identify a healthy broker-backed dashboard without requiring extra flags. +- Remaining warnings are the pre-existing `websockets` / `uvicorn` + deprecations already seen in the repository test suite. diff --git a/src/mcpbridge_wrapper/__main__.py b/src/mcpbridge_wrapper/__main__.py index c465c93..08874a3 100644 --- a/src/mcpbridge_wrapper/__main__.py +++ b/src/mcpbridge_wrapper/__main__.py @@ -156,6 +156,20 @@ def _parse_broker_console_args(args: list) -> Tuple[bool, list]: return broker_console, remaining +def _parse_doctor_args(args: list) -> Tuple[bool, list]: + """Parse diagnostics mode arguments from command-line args.""" + doctor_enabled = False + remaining = [] + + for arg in args: + if arg == "--doctor": + doctor_enabled = True + else: + remaining.append(arg) + + return doctor_enabled, remaining + + def _find_listener_pids_for_port(port: int) -> Set[int]: """Return listener PIDs bound to TCP port, or empty set when none found.""" try: @@ -705,6 +719,7 @@ def main() -> int: Supports optional --web-ui flag to start a monitoring dashboard. Supports optional --tui flag for standalone broker terminal monitoring. + Supports optional --doctor flag for broker workflow diagnostics. Supports optional --broker-daemon flag to start a persistent broker host. Supports optional --broker flag for proxy mode. @@ -728,8 +743,9 @@ def main() -> int: tui_enabled, after_tui_args = _parse_tui_args(after_webui_args) broker_console, after_console_args = _parse_broker_console_args(after_tui_args) + doctor_enabled, after_doctor_args = _parse_doctor_args(after_console_args) broker_daemon, broker_connect, broker_spawn, broker_status, broker_stop, bridge_args = ( - _parse_broker_args(after_console_args) + _parse_broker_args(after_doctor_args) ) if tui_enabled and broker_console: @@ -766,6 +782,29 @@ def main() -> int: print("Error: --broker-console does not accept bridge arguments.", file=sys.stderr) return 2 + if doctor_enabled and web_ui_enabled: + print( + "Error: --doctor cannot be combined with --web-ui flags. " + "Use --web-ui-port/--web-ui-config to target an existing dashboard.", + file=sys.stderr, + ) + return 2 + + if doctor_enabled and (tui_enabled or broker_console): + print( + "Error: --doctor cannot be combined with --tui or --broker-console.", + file=sys.stderr, + ) + return 2 + + if doctor_enabled and (broker_daemon or broker_connect or broker_status or broker_stop): + print("Error: --doctor cannot be combined with broker mode flags.", file=sys.stderr) + return 2 + + if doctor_enabled and bridge_args: + print("Error: --doctor does not accept bridge arguments.", file=sys.stderr) + return 2 + if web_ui_only and (broker_console or broker_daemon or broker_connect): print( "Error: --web-ui-only cannot be combined with broker mode flags.", @@ -799,6 +838,14 @@ def main() -> int: web_ui_restart=web_ui_restart, ) + if doctor_enabled: + from mcpbridge_wrapper.doctor import run_doctor + + return run_doctor( + web_ui_port=web_ui_port, + web_ui_config=web_ui_config, + ) + # --broker-status: print broker daemon status and exit if broker_status: from mcpbridge_wrapper import __version__ diff --git a/src/mcpbridge_wrapper/doctor.py b/src/mcpbridge_wrapper/doctor.py new file mode 100644 index 0000000..03c3b62 --- /dev/null +++ b/src/mcpbridge_wrapper/doctor.py @@ -0,0 +1,580 @@ +"""User-facing broker diagnostics for the dedicated-host workflow.""" + +from __future__ import annotations + +import contextlib +import os +import shutil +import subprocess +import sys +from dataclasses import dataclass, field +from typing import Any +from urllib.parse import urlparse + +from mcpbridge_wrapper import __version__ +from mcpbridge_wrapper.tui import BrokerTUIClient, TUIRuntimeConfig, build_tui_runtime + + +@dataclass +class LocalBrokerDiagnostics: + """Observed local broker state from PID/socket/version files.""" + + pid_file: str + pid_file_present: bool + pid: int | None + pid_running: bool + socket_path: str + socket_present: bool + version_file: str + version_file_present: bool + version: str | None + version_mismatch: bool + + +@dataclass +class DashboardDiagnostics: + """Observed dashboard endpoint state and ownership details.""" + + base_url: str + port: int | None + listener_pids: list[int] = field(default_factory=list) + listener_commands: dict[int, str] = field(default_factory=dict) + health_ok: bool = False + health_error: str | None = None + control: dict[str, Any] | None = None + broker_status: dict[str, Any] | None = None + backend_error: str | None = None + + @property + def service_name(self) -> str | None: + """Return the reported service name when the endpoint exposes one.""" + for payload in (self.broker_status, self.control): + if not isinstance(payload, dict): + continue + service_name = payload.get("service_name") + if isinstance(service_name, str) and service_name: + return service_name + return None + + @property + def broker_payload(self) -> dict[str, Any] | None: + """Return structured broker payload when exposed by the endpoint.""" + if not isinstance(self.broker_status, dict): + return None + payload = self.broker_status.get("broker") + return payload if isinstance(payload, dict) else None + + @property + def broker_available(self) -> bool: + """Return whether the endpoint reports broker runtime availability.""" + if not isinstance(self.broker_status, dict): + return False + return bool(self.broker_status.get("available")) + + +@dataclass +class DoctorReport: + """Rendered doctor result for CLI output and exit-code decisions.""" + + code: str + ok: bool + summary: str + next_action: str + exit_code: int + python_runtime_lines: list[str] + local_state_lines: list[str] + dashboard_lines: list[str] + broker_runtime_lines: list[str] + evidence_lines: list[str] + + +def _pid_exists(pid: int) -> bool: + """Return True when the process exists and is probeable.""" + try: + os.kill(pid, 0) + except ProcessLookupError: + return False + except PermissionError: + return True + return True + + +def _read_local_pid(pid_file: str) -> tuple[int | None, bool]: + """Read a broker PID file and report whether the process is still alive.""" + if not os.path.exists(pid_file): + return None, False + + try: + with open(pid_file, encoding="utf-8") as handle: + pid = int(handle.read().strip()) + except (OSError, ValueError): + return None, False + + return pid, _pid_exists(pid) + + +def _read_local_version(version_file: str) -> str | None: + """Read the local broker version stamp when present.""" + if not os.path.exists(version_file): + return None + + try: + with open(version_file, encoding="utf-8") as handle: + version = handle.read().strip() + except OSError: + return None + + return version or None + + +def _find_listener_pids_for_port(port: int | None) -> list[int]: + """Return listener PIDs bound to a TCP port, or an empty list.""" + if port is None: + return [] + + try: + result = subprocess.run( + ["lsof", f"-tiTCP:{port}", "-sTCP:LISTEN"], + capture_output=True, + text=True, + check=False, + ) + except OSError: + return [] + + pids: list[int] = [] + for raw in result.stdout.splitlines(): + raw = raw.strip() + if not raw: + continue + with contextlib.suppress(ValueError): + pids.append(int(raw)) + return sorted(set(pids)) + + +def _read_process_command(pid: int) -> str | None: + """Return the command line for a PID when available.""" + try: + command = subprocess.check_output( + ["ps", "-p", str(pid), "-o", "command="], + stderr=subprocess.DEVNULL, + text=True, + ).strip() + except (OSError, subprocess.CalledProcessError): + return None + return command or None + + +def collect_local_broker_diagnostics(runtime: TUIRuntimeConfig) -> LocalBrokerDiagnostics: + """Collect PID/socket/version state from the local broker directory.""" + pid_file = str(runtime.pid_file) + pid, pid_running = _read_local_pid(pid_file) + version = _read_local_version(str(runtime.version_file)) + + return LocalBrokerDiagnostics( + pid_file=pid_file, + pid_file_present=runtime.pid_file.exists(), + pid=pid, + pid_running=pid_running, + socket_path=str(runtime.socket_path), + socket_present=runtime.socket_path.exists(), + version_file=str(runtime.version_file), + version_file_present=runtime.version_file.exists(), + version=version, + version_mismatch=bool(version and version != __version__), + ) + + +def collect_dashboard_diagnostics(runtime: TUIRuntimeConfig) -> DashboardDiagnostics: + """Probe the configured dashboard endpoint and any local listener ownership.""" + port = urlparse(runtime.base_url).port + listener_pids = _find_listener_pids_for_port(port) + listener_commands: dict[int, str] = {} + for pid in listener_pids: + command = _read_process_command(pid) + if command is not None: + listener_commands[pid] = command + + client = BrokerTUIClient(runtime) + diagnostics = DashboardDiagnostics( + base_url=runtime.base_url, + port=port, + listener_pids=listener_pids, + listener_commands=listener_commands, + ) + + try: + health_payload = client._request_json("/api/health") + except RuntimeError as exc: + diagnostics.health_error = str(exc) + else: + diagnostics.health_ok = health_payload.get("status") == "ok" + if not diagnostics.health_ok: + diagnostics.health_error = "GET /api/health returned a non-ok status." + + try: + control, broker_status = client.probe_backend() + except RuntimeError as exc: + diagnostics.backend_error = str(exc) + else: + diagnostics.control = control + diagnostics.broker_status = broker_status + + return diagnostics + + +def classify_doctor_report( + runtime: TUIRuntimeConfig, + local: LocalBrokerDiagnostics, + dashboard: DashboardDiagnostics, +) -> DoctorReport: + """Classify the current runtime into one actionable user-facing diagnosis.""" + python_runtime_lines = [ + f"Package Version: {__version__}", + f"Python Executable: {sys.executable}", + f"xcrun: {shutil.which('xcrun') or 'not found'}", + ] + local_state_lines = [ + f"PID File: {local.pid_file} ({'present' if local.pid_file_present else 'missing'})", + "Daemon PID: " + + ( + f"{local.pid} ({'running' if local.pid_running else 'not running'})" + if local.pid is not None + else "n/a" + ), + f"Socket: {local.socket_path} ({'present' if local.socket_present else 'missing'})", + "Version File: " + + f"{local.version_file} ({'present' if local.version_file_present else 'missing'})", + f"Recorded Daemon Version: {local.version or 'n/a'}", + ] + if local.version_mismatch: + local_state_lines.append(f"Version Mismatch: yes (package {__version__})") + else: + local_state_lines.append("Version Mismatch: no") + + dashboard_lines = [ + f"Endpoint: {dashboard.base_url}", + f"Port: {dashboard.port if dashboard.port is not None else 'n/a'}", + "Listener PIDs: " + (", ".join(str(pid) for pid in dashboard.listener_pids) or "none"), + f"Health Check: {'ok' if dashboard.health_ok else 'unreachable'}", + ] + for pid, command in dashboard.listener_commands.items(): + dashboard_lines.append(f"Listener {pid}: {command}") + if dashboard.service_name: + dashboard_lines.append(f"Reported Service: {dashboard.service_name}") + if dashboard.health_error: + dashboard_lines.append(f"Health Detail: {dashboard.health_error}") + if dashboard.backend_error: + dashboard_lines.append(f"Backend Detail: {dashboard.backend_error}") + + broker_runtime_lines: list[str] = [] + evidence_lines: list[str] = [] + + if local.version_mismatch and local.pid_running: + return DoctorReport( + code="version-mismatch", + ok=False, + summary=( + "Running broker daemon version does not match this mcpbridge-wrapper installation." + ), + next_action=( + "Restart the broker from this environment with " + "`mcpbridge-wrapper --broker-stop && mcpbridge-wrapper --broker-console`." + ), + exit_code=1, + python_runtime_lines=python_runtime_lines, + local_state_lines=local_state_lines, + dashboard_lines=dashboard_lines, + broker_runtime_lines=broker_runtime_lines, + evidence_lines=[ + f"Daemon version file reports {local.version}, package version is {__version__}.", + ], + ) + + if ( + local.pid_file_present + and not local.pid_running + and (local.socket_present or local.version_file_present) + ): + evidence_lines.append("PID file exists but does not point to a running broker process.") + if local.socket_present: + evidence_lines.append("Broker socket still exists on disk.") + if local.version_file_present: + evidence_lines.append("Broker version stamp still exists on disk.") + return DoctorReport( + code="stale-runtime", + ok=False, + summary=( + "Broker state files are stale; no live broker process owns the current runtime." + ), + next_action=( + "Clean up with `mcpbridge-wrapper --broker-stop`, then restart with " + "`mcpbridge-wrapper --broker-console`." + ), + exit_code=1, + python_runtime_lines=python_runtime_lines, + local_state_lines=local_state_lines, + dashboard_lines=dashboard_lines, + broker_runtime_lines=broker_runtime_lines, + evidence_lines=evidence_lines, + ) + + if dashboard.broker_available and dashboard.broker_payload is not None: + broker_payload = dashboard.broker_payload + broker_runtime_lines = [ + f"State: {broker_payload.get('state', 'n/a')}", + f"Daemon PID: {broker_payload.get('pid', 'n/a')}", + f"Upstream PID: {broker_payload.get('upstream_pid', 'n/a')}", + f"Upstream Alive: {'yes' if broker_payload.get('upstream_alive') else 'no'}", + f"Initialized: {'yes' if broker_payload.get('upstream_initialized') else 'no'}", + f"Connected Clients: {broker_payload.get('connected_clients', 'n/a')}", + f"Reconnect Attempt: {broker_payload.get('reconnect_attempt', 'n/a')}", + ] + state = str(broker_payload.get("state") or "") + upstream_alive = bool(broker_payload.get("upstream_alive")) + upstream_initialized = bool(broker_payload.get("upstream_initialized")) + + if state == "ready" and upstream_alive and upstream_initialized: + return DoctorReport( + code="healthy", + ok=True, + summary="Broker daemon and broker-backed dashboard are healthy.", + next_action=( + "Connect your MCP client with `--broker`, or use " + "`mcpbridge-wrapper --broker-console` for the attached frontend." + ), + exit_code=0, + python_runtime_lines=python_runtime_lines, + local_state_lines=local_state_lines, + dashboard_lines=dashboard_lines, + broker_runtime_lines=broker_runtime_lines, + evidence_lines=["Dashboard is reachable and reports service `broker-daemon`."], + ) + + if not upstream_alive: + evidence_lines.append( + "The broker-backed dashboard is reachable, but the upstream " + "Xcode bridge is not alive." + ) + if not upstream_initialized: + evidence_lines.append("The upstream initialize/tools probe has not completed yet.") + if state and state != "ready": + evidence_lines.append(f"Broker runtime state is `{state}`.") + return DoctorReport( + code="broker-degraded", + ok=False, + summary=( + "Broker-backed dashboard is reachable, but the broker runtime is not fully ready." + ), + next_action=( + "Keep Xcode open with Xcode Tools enabled and retry. If the state stays degraded, " + "restart with `mcpbridge-wrapper --broker-console`." + ), + exit_code=1, + python_runtime_lines=python_runtime_lines, + local_state_lines=local_state_lines, + dashboard_lines=dashboard_lines, + broker_runtime_lines=broker_runtime_lines, + evidence_lines=evidence_lines, + ) + + if dashboard.service_name == "broker-daemon": + status_error = None + if isinstance(dashboard.broker_status, dict): + raw_error = dashboard.broker_status.get("error") + if isinstance(raw_error, str) and raw_error: + status_error = raw_error + if dashboard.backend_error: + evidence_lines.append(dashboard.backend_error) + if status_error: + evidence_lines.append(status_error) + return DoctorReport( + code="broker-degraded", + ok=False, + summary=( + "Broker-backed dashboard is reachable, but broker runtime status is unavailable." + ), + next_action=( + "Retry after Xcode finishes any approval flow. If the runtime stays unavailable, " + "restart with `mcpbridge-wrapper --broker-console`." + ), + exit_code=1, + python_runtime_lines=python_runtime_lines, + local_state_lines=local_state_lines, + dashboard_lines=dashboard_lines, + broker_runtime_lines=broker_runtime_lines, + evidence_lines=evidence_lines + or ["Dashboard reports service `broker-daemon` but did not return runtime status."], + ) + + if dashboard.service_name and dashboard.service_name != "broker-daemon": + evidence_lines.append( + f"Endpoint reports service `{dashboard.service_name}` instead of `broker-daemon`." + ) + if dashboard.listener_pids: + evidence_lines.append( + "Port owner PIDs: " + ", ".join(str(pid) for pid in dashboard.listener_pids) + ) + return DoctorReport( + code="wrong-service", + ok=False, + summary=( + f"Dashboard endpoint {dashboard.base_url} is alive, " + "but it is not the dedicated broker host." + ), + next_action=( + "Stop the existing listener or retry startup with " + "`mcpbridge-wrapper --broker-console --web-ui-restart`." + ), + exit_code=1, + python_runtime_lines=python_runtime_lines, + local_state_lines=local_state_lines, + dashboard_lines=dashboard_lines, + broker_runtime_lines=broker_runtime_lines, + evidence_lines=evidence_lines, + ) + + if dashboard.listener_pids and (dashboard.health_ok or dashboard.backend_error): + evidence_lines.append( + "A listener already owns the configured dashboard port, but it is " + "not exposing broker-daemon." + ) + if dashboard.backend_error: + evidence_lines.append(dashboard.backend_error) + return DoctorReport( + code="port-occupied", + ok=False, + summary=f"Dashboard port {dashboard.port} is occupied by another listener.", + next_action=( + "Free the port or retry startup with " + "`mcpbridge-wrapper --broker-console --web-ui-restart`." + ), + exit_code=1, + python_runtime_lines=python_runtime_lines, + local_state_lines=local_state_lines, + dashboard_lines=dashboard_lines, + broker_runtime_lines=broker_runtime_lines, + evidence_lines=evidence_lines, + ) + + if local.pid_running: + if dashboard.health_error: + evidence_lines.append(dashboard.health_error) + if dashboard.backend_error: + evidence_lines.append(dashboard.backend_error) + return DoctorReport( + code="broker-without-dashboard", + ok=False, + summary=( + "Broker daemon is running, but no broker-backed dashboard is " + f"reachable at {runtime.base_url}." + ), + next_action=( + "Restart the dedicated host with " + "`mcpbridge-wrapper --broker-stop && mcpbridge-wrapper --broker-console`." + ), + exit_code=1, + python_runtime_lines=python_runtime_lines, + local_state_lines=local_state_lines, + dashboard_lines=dashboard_lines, + broker_runtime_lines=broker_runtime_lines, + evidence_lines=evidence_lines or ["No reachable broker-backed dashboard was detected."], + ) + + if dashboard.listener_pids: + evidence_lines.append( + "A process is already listening on the configured dashboard port before broker startup." + ) + return DoctorReport( + code="port-occupied", + ok=False, + summary=f"Dashboard port {dashboard.port} is already occupied before broker startup.", + next_action=( + "Stop the existing listener or start again with " + "`mcpbridge-wrapper --broker-console --web-ui-restart`." + ), + exit_code=1, + python_runtime_lines=python_runtime_lines, + local_state_lines=local_state_lines, + dashboard_lines=dashboard_lines, + broker_runtime_lines=broker_runtime_lines, + evidence_lines=evidence_lines, + ) + + if dashboard.health_error: + evidence_lines.append(dashboard.health_error) + if dashboard.backend_error: + evidence_lines.append(dashboard.backend_error) + + return DoctorReport( + code="broker-not-running", + ok=False, + summary="No running broker daemon was detected.", + next_action=( + "Start the recommended dedicated host with `mcpbridge-wrapper --broker-console`." + ), + exit_code=1, + python_runtime_lines=python_runtime_lines, + local_state_lines=local_state_lines, + dashboard_lines=dashboard_lines, + broker_runtime_lines=broker_runtime_lines, + evidence_lines=( + evidence_lines or ["No live broker PID or broker-backed dashboard was found."] + ), + ) + + +def collect_doctor_report( + *, + web_ui_port: int | None, + web_ui_config: str | None, +) -> DoctorReport: + """Collect and classify broker diagnostics for the configured endpoint.""" + runtime = build_tui_runtime( + web_ui_port=web_ui_port, + web_ui_config=web_ui_config, + ) + local = collect_local_broker_diagnostics(runtime) + dashboard = collect_dashboard_diagnostics(runtime) + return classify_doctor_report(runtime, local, dashboard) + + +def render_doctor_report(report: DoctorReport) -> str: + """Render a user-facing diagnostics report.""" + lines = [ + "mcpbridge-wrapper doctor", + f"Status: {'OK' if report.ok else 'ISSUE'}", + f"Summary: {report.summary}", + f"Next Action: {report.next_action}", + "", + "Python Runtime", + *[f"- {line}" for line in report.python_runtime_lines], + "", + "Local Broker State", + *[f"- {line}" for line in report.local_state_lines], + "", + "Dashboard", + *[f"- {line}" for line in report.dashboard_lines], + ] + + if report.broker_runtime_lines: + lines.extend(["", "Broker Runtime", *[f"- {line}" for line in report.broker_runtime_lines]]) + + if report.evidence_lines: + lines.extend(["", "Evidence", *[f"- {line}" for line in report.evidence_lines]]) + + return "\n".join(lines) + + +def run_doctor( + *, + web_ui_port: int | None, + web_ui_config: str | None, +) -> int: + """Execute doctor mode and print the resulting report.""" + report = collect_doctor_report( + web_ui_port=web_ui_port, + web_ui_config=web_ui_config, + ) + print(render_doctor_report(report)) + return report.exit_code diff --git a/tests/unit/test_doctor.py b/tests/unit/test_doctor.py new file mode 100644 index 0000000..77a3e74 --- /dev/null +++ b/tests/unit/test_doctor.py @@ -0,0 +1,558 @@ +"""Tests for broker doctor diagnostics.""" + +from __future__ import annotations + +from pathlib import Path +from unittest.mock import patch + +from mcpbridge_wrapper.doctor import ( + DashboardDiagnostics, + DoctorReport, + LocalBrokerDiagnostics, + _find_listener_pids_for_port, + _pid_exists, + _read_local_pid, + _read_local_version, + _read_process_command, + classify_doctor_report, + collect_dashboard_diagnostics, + collect_doctor_report, + collect_local_broker_diagnostics, + render_doctor_report, + run_doctor, +) +from mcpbridge_wrapper.tui import TUIRuntimeConfig + + +def _runtime(base_url: str = "http://127.0.0.1:8080") -> TUIRuntimeConfig: + return TUIRuntimeConfig( + base_url=base_url, + auth_header=None, + log_path=Path("/tmp/broker.log"), + pid_file=Path("/tmp/broker.pid"), + socket_path=Path("/tmp/broker.sock"), + version_file=Path("/tmp/broker.version"), + ) + + +def _local( + *, + pid_file_present: bool = True, + pid: int | None = 100, + pid_running: bool = True, + socket_present: bool = True, + version_file_present: bool = True, + version: str | None = "0.4.1", + version_mismatch: bool = False, +) -> LocalBrokerDiagnostics: + return LocalBrokerDiagnostics( + pid_file="/tmp/broker.pid", + pid_file_present=pid_file_present, + pid=pid, + pid_running=pid_running, + socket_path="/tmp/broker.sock", + socket_present=socket_present, + version_file="/tmp/broker.version", + version_file_present=version_file_present, + version=version, + version_mismatch=version_mismatch, + ) + + +def _dashboard( + *, + health_ok: bool = False, + health_error: str | None = "Cannot reach http://127.0.0.1:8080: [Errno 61] Connection refused", + listener_pids: list[int] | None = None, + listener_commands: dict[int, str] | None = None, + control: dict | None = None, + broker_status: dict | None = None, + backend_error: str | None = "GET /api/control failed: Not Found", +) -> DashboardDiagnostics: + return DashboardDiagnostics( + base_url="http://127.0.0.1:8080", + port=8080, + listener_pids=list(listener_pids or []), + listener_commands=dict(listener_commands or {}), + health_ok=health_ok, + health_error=health_error, + control=control, + broker_status=broker_status, + backend_error=backend_error, + ) + + +class TestDoctorHelpers: + """Tests for low-level doctor helpers and data accessors.""" + + def test_dashboard_properties_surface_service_and_broker_payload(self) -> None: + dashboard = DashboardDiagnostics( + base_url="http://127.0.0.1:8080", + port=8080, + control={"service_name": "broker-daemon", "can_stop": True}, + broker_status={ + "available": True, + "service_name": "broker-daemon", + "broker": {"state": "ready"}, + }, + ) + + assert dashboard.service_name == "broker-daemon" + assert dashboard.broker_available is True + assert dashboard.broker_payload == {"state": "ready"} + + def test_pid_exists_handles_lookup_and_permission_errors(self) -> None: + with patch("mcpbridge_wrapper.doctor.os.kill", return_value=None): + assert _pid_exists(123) is True + + with patch("mcpbridge_wrapper.doctor.os.kill", side_effect=ProcessLookupError): + assert _pid_exists(123) is False + + with patch("mcpbridge_wrapper.doctor.os.kill", side_effect=PermissionError): + assert _pid_exists(123) is True + + def test_read_local_pid_handles_missing_invalid_and_running_pid(self, tmp_path: Path) -> None: + missing = tmp_path / "missing.pid" + assert _read_local_pid(str(missing)) == (None, False) + + invalid = tmp_path / "invalid.pid" + invalid.write_text("not-a-pid") + assert _read_local_pid(str(invalid)) == (None, False) + + valid = tmp_path / "broker.pid" + valid.write_text("321") + with patch("mcpbridge_wrapper.doctor._pid_exists", return_value=True): + assert _read_local_pid(str(valid)) == (321, True) + + def test_read_local_version_handles_missing_error_and_empty(self, tmp_path: Path) -> None: + missing = tmp_path / "missing.version" + assert _read_local_version(str(missing)) is None + + empty = tmp_path / "empty.version" + empty.write_text("") + assert _read_local_version(str(empty)) is None + + broken = tmp_path / "broken.version" + broken.write_text("0.4.1") + with patch("builtins.open", side_effect=OSError("denied")): + assert _read_local_version(str(broken)) is None + + def test_find_listener_pids_for_port_handles_none_duplicates_and_oserror(self) -> None: + assert _find_listener_pids_for_port(None) == [] + + completed = type("Completed", (), {"stdout": "123\n456\n123\njunk\n"})() + with patch("mcpbridge_wrapper.doctor.subprocess.run", return_value=completed): + assert _find_listener_pids_for_port(8080) == [123, 456] + + with patch("mcpbridge_wrapper.doctor.subprocess.run", side_effect=OSError): + assert _find_listener_pids_for_port(8080) == [] + + def test_read_process_command_handles_success_and_failure(self) -> None: + with patch( + "mcpbridge_wrapper.doctor.subprocess.check_output", + return_value="python -m mcpbridge_wrapper --broker-daemon\n", + ): + assert _read_process_command(777) == "python -m mcpbridge_wrapper --broker-daemon" + + with patch( + "mcpbridge_wrapper.doctor.subprocess.check_output", + side_effect=OSError("ps failed"), + ): + assert _read_process_command(777) is None + + def test_collect_local_broker_diagnostics_reads_runtime_files(self, tmp_path: Path) -> None: + pid_file = tmp_path / "broker.pid" + pid_file.write_text("654") + socket_path = tmp_path / "broker.sock" + socket_path.write_text("sock") + version_file = tmp_path / "broker.version" + version_file.write_text("0.4.1") + runtime = TUIRuntimeConfig( + base_url="http://127.0.0.1:8080", + auth_header=None, + log_path=tmp_path / "broker.log", + pid_file=pid_file, + socket_path=socket_path, + version_file=version_file, + ) + + with patch("mcpbridge_wrapper.doctor._pid_exists", return_value=True): + diagnostics = collect_local_broker_diagnostics(runtime) + + assert diagnostics.pid == 654 + assert diagnostics.pid_running is True + assert diagnostics.socket_present is True + assert diagnostics.version == "0.4.1" + + def test_collect_dashboard_diagnostics_collects_successful_probes(self) -> None: + runtime = _runtime() + + with patch( + "mcpbridge_wrapper.doctor._find_listener_pids_for_port", + return_value=[111], + ), patch( + "mcpbridge_wrapper.doctor._read_process_command", + return_value="python -m mcpbridge_wrapper --broker-daemon", + ), patch( + "mcpbridge_wrapper.tui.BrokerTUIClient._request_json", + return_value={"status": "ok"}, + ), patch( + "mcpbridge_wrapper.tui.BrokerTUIClient.probe_backend", + return_value=( + {"service_name": "broker-daemon", "can_stop": True}, + {"available": True, "service_name": "broker-daemon", "broker": {"state": "ready"}}, + ), + ): + diagnostics = collect_dashboard_diagnostics(runtime) + + assert diagnostics.listener_pids == [111] + assert diagnostics.listener_commands[111] == "python -m mcpbridge_wrapper --broker-daemon" + assert diagnostics.health_ok is True + assert diagnostics.backend_error is None + assert diagnostics.service_name == "broker-daemon" + + def test_collect_dashboard_diagnostics_handles_failed_probes(self) -> None: + runtime = _runtime() + + with patch( + "mcpbridge_wrapper.doctor._find_listener_pids_for_port", + return_value=[], + ), patch( + "mcpbridge_wrapper.tui.BrokerTUIClient._request_json", + side_effect=RuntimeError("health down"), + ), patch( + "mcpbridge_wrapper.tui.BrokerTUIClient.probe_backend", + side_effect=RuntimeError("backend down"), + ): + diagnostics = collect_dashboard_diagnostics(runtime) + + assert diagnostics.health_ok is False + assert diagnostics.health_error == "health down" + assert diagnostics.backend_error == "backend down" + + +class TestClassifyDoctorReport: + """Tests for doctor diagnosis classification.""" + + def test_classify_version_mismatch(self) -> None: + report = classify_doctor_report( + _runtime(), + _local(version="0.0.1-old", version_mismatch=True), + _dashboard(listener_pids=[], backend_error=None), + ) + + assert report.ok is False + assert report.code == "version-mismatch" + assert "does not match" in report.summary.lower() + + def test_classify_healthy_broker_backed_runtime(self) -> None: + dashboard = _dashboard( + health_ok=True, + health_error=None, + backend_error=None, + control={"service_name": "broker-daemon", "can_stop": True}, + broker_status={ + "available": True, + "service_name": "broker-daemon", + "broker": { + "state": "ready", + "pid": 100, + "upstream_pid": 101, + "upstream_alive": True, + "upstream_initialized": True, + "connected_clients": 2, + "reconnect_attempt": 0, + }, + }, + ) + + report = classify_doctor_report(_runtime(), _local(), dashboard) + + assert report.ok is True + assert report.code == "healthy" + assert "healthy" in report.summary.lower() + assert "--broker" in report.next_action + + def test_classify_broker_backed_runtime_not_ready(self) -> None: + dashboard = _dashboard( + health_ok=True, + health_error=None, + backend_error=None, + control={"service_name": "broker-daemon", "can_stop": True}, + broker_status={ + "available": True, + "service_name": "broker-daemon", + "broker": { + "state": "reconnecting", + "pid": 100, + "upstream_pid": 101, + "upstream_alive": False, + "upstream_initialized": False, + "connected_clients": 0, + "reconnect_attempt": 2, + }, + }, + ) + + report = classify_doctor_report(_runtime(), _local(), dashboard) + + assert report.ok is False + assert report.code == "broker-degraded" + assert any("not alive" in line for line in report.evidence_lines) + assert any("reconnecting" in line for line in report.evidence_lines) + + def test_classify_broker_running_without_dashboard(self) -> None: + report = classify_doctor_report(_runtime(), _local(), _dashboard(listener_pids=[])) + + 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 + + def test_classify_wrong_service_on_dashboard_port(self) -> None: + dashboard = _dashboard( + health_ok=True, + health_error=None, + listener_pids=[501], + listener_commands={501: "python -m http.server 8080"}, + control={"service_name": "mcpbridge-wrapper", "can_stop": False}, + broker_status={"available": False, "service_name": "mcpbridge-wrapper", "broker": None}, + backend_error=None, + ) + + report = classify_doctor_report( + _runtime(), + _local( + pid_file_present=False, + pid=None, + pid_running=False, + socket_present=False, + version_file_present=False, + version=None, + ), + dashboard, + ) + + 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 + + def test_classify_broker_daemon_with_unavailable_runtime_status(self) -> None: + dashboard = _dashboard( + health_ok=True, + health_error=None, + control={"service_name": "broker-daemon", "can_stop": True}, + broker_status={ + "available": False, + "service_name": "broker-daemon", + "broker": None, + "error": "upstream reconnecting", + }, + backend_error=None, + ) + + report = classify_doctor_report(_runtime(), _local(), dashboard) + + assert report.ok is False + assert report.code == "broker-degraded" + assert "runtime status is unavailable" in report.summary.lower() + assert any("upstream reconnecting" in line for line in report.evidence_lines) + + def test_classify_stale_runtime_files(self) -> None: + report = classify_doctor_report( + _runtime(), + _local(pid_running=False, socket_present=True, version_file_present=True), + _dashboard(listener_pids=[], backend_error=None), + ) + + assert report.ok is False + assert report.code == "stale-runtime" + assert "stale" in report.summary.lower() + assert "--broker-stop" in report.next_action + + def test_classify_port_occupied_before_broker_startup(self) -> None: + dashboard = _dashboard( + health_ok=False, + listener_pids=[777], + listener_commands={777: "node /tmp/other-service.js"}, + control=None, + broker_status=None, + backend_error="GET /api/control failed: Not Found", + ) + + report = classify_doctor_report( + _runtime(), + _local(pid_file_present=False, pid=None, pid_running=False, socket_present=False), + dashboard, + ) + + assert report.ok is False + assert report.code == "port-occupied" + assert "occupied" in report.summary.lower() + assert "--web-ui-restart" in report.next_action + + def test_classify_broker_not_running_without_any_endpoint(self) -> None: + report = classify_doctor_report( + _runtime(), + _local( + pid_file_present=False, + pid=None, + pid_running=False, + socket_present=False, + version_file_present=False, + version=None, + ), + _dashboard( + listener_pids=[], + health_error=None, + backend_error=None, + ), + ) + + assert report.ok is False + assert report.code == "broker-not-running" + assert "no running broker daemon" in report.summary.lower() + + +class TestRenderDoctorReport: + """Tests for doctor output rendering.""" + + def test_render_report_includes_summary_sections(self) -> None: + report = DoctorReport( + code="broker-not-running", + ok=False, + summary="No running broker daemon was detected.", + next_action="Start with `mcpbridge-wrapper --broker-console`.", + exit_code=1, + python_runtime_lines=["Package Version: 0.4.1"], + local_state_lines=["PID File: /tmp/broker.pid (missing)"], + dashboard_lines=["Endpoint: http://127.0.0.1:8080"], + broker_runtime_lines=[], + evidence_lines=["No live broker PID or broker-backed dashboard was found."], + ) + + rendered = render_doctor_report(report) + + assert "mcpbridge-wrapper doctor" in rendered + assert "Status: ISSUE" in rendered + assert "Next Action:" in rendered + assert "Python Runtime" in rendered + assert "Evidence" in rendered + + def test_render_report_marks_healthy_status(self) -> None: + report = DoctorReport( + code="healthy", + ok=True, + summary="Broker daemon and broker-backed dashboard are healthy.", + next_action="Connect with `--broker`.", + exit_code=0, + python_runtime_lines=["Package Version: 0.4.1"], + local_state_lines=["Daemon PID: 100 (running)"], + dashboard_lines=["Endpoint: http://127.0.0.1:8080"], + broker_runtime_lines=["State: ready"], + evidence_lines=["Dashboard is reachable and reports service `broker-daemon`."], + ) + + rendered = render_doctor_report(report) + + assert "Status: OK" in rendered + assert "Broker Runtime" in rendered + assert "State: ready" in rendered + + +class TestCollectDoctorReport: + """Light integration tests for doctor collection helpers.""" + + def test_collect_dashboard_runtime_report_uses_live_version(self) -> None: + runtime = _runtime() + dashboard = _dashboard( + health_ok=True, + health_error=None, + backend_error=None, + control={"service_name": "broker-daemon", "can_stop": True}, + broker_status={ + "available": True, + "service_name": "broker-daemon", + "broker": { + "state": "ready", + "pid": 100, + "upstream_pid": 101, + "upstream_alive": True, + "upstream_initialized": True, + "connected_clients": 0, + "reconnect_attempt": 0, + }, + }, + ) + + with patch("mcpbridge_wrapper.doctor.__version__", "9.9.9"): + report = classify_doctor_report( + runtime, + _local(version="9.9.9", version_mismatch=False), + dashboard, + ) + + assert report.ok is True + assert any("Package Version: 9.9.9" in line for line in report.python_runtime_lines) + + def test_collect_doctor_report_uses_runtime_and_collectors(self) -> None: + runtime = _runtime(base_url="http://127.0.0.1:9191") + local = _local() + dashboard = _dashboard( + health_ok=True, + health_error=None, + backend_error=None, + control={"service_name": "broker-daemon", "can_stop": True}, + broker_status={ + "available": True, + "service_name": "broker-daemon", + "broker": { + "state": "ready", + "pid": 100, + "upstream_pid": 101, + "upstream_alive": True, + "upstream_initialized": True, + "connected_clients": 0, + "reconnect_attempt": 0, + }, + }, + ) + + with patch("mcpbridge_wrapper.doctor.build_tui_runtime", return_value=runtime), patch( + "mcpbridge_wrapper.doctor.collect_local_broker_diagnostics", + return_value=local, + ) as collect_local, patch( + "mcpbridge_wrapper.doctor.collect_dashboard_diagnostics", + return_value=dashboard, + ) as collect_dashboard: + report = collect_doctor_report(web_ui_port=9191, web_ui_config=None) + + assert report.ok is True + collect_local.assert_called_once_with(runtime) + collect_dashboard.assert_called_once_with(runtime) + + def test_run_doctor_prints_rendered_report_and_returns_exit_code(self) -> None: + report = DoctorReport( + code="broker-not-running", + ok=False, + summary="No running broker daemon was detected.", + next_action="Start with `mcpbridge-wrapper --broker-console`.", + exit_code=1, + python_runtime_lines=["Package Version: 0.4.1"], + local_state_lines=["PID File: /tmp/broker.pid (missing)"], + dashboard_lines=["Endpoint: http://127.0.0.1:8080"], + broker_runtime_lines=[], + evidence_lines=["No live broker PID or broker-backed dashboard was found."], + ) + + with patch("mcpbridge_wrapper.doctor.collect_doctor_report", return_value=report), patch( + "builtins.print" + ) as print_mock: + exit_code = run_doctor(web_ui_port=None, web_ui_config=None) + + assert exit_code == 1 + print_mock.assert_called_once() + rendered = print_mock.call_args.args[0] + assert "mcpbridge-wrapper doctor" in rendered diff --git a/tests/unit/test_main_doctor.py b/tests/unit/test_main_doctor.py new file mode 100644 index 0000000..dd74245 --- /dev/null +++ b/tests/unit/test_main_doctor.py @@ -0,0 +1,80 @@ +"""Tests for __main__.py doctor integration.""" + +from unittest.mock import patch + +from mcpbridge_wrapper.__main__ import _parse_doctor_args, main + + +class TestParseDoctorArgs: + """Tests for _parse_doctor_args.""" + + def test_parse_doctor_flag(self) -> None: + enabled, remaining = _parse_doctor_args(["--doctor", "--foo"]) + + assert enabled is True + assert remaining == ["--foo"] + + def test_parse_doctor_flag_absent(self) -> None: + enabled, remaining = _parse_doctor_args(["--foo"]) + + assert enabled is False + assert remaining == ["--foo"] + + +class TestMainDoctor: + """Tests for main() behavior in doctor mode.""" + + def test_main_doctor_dispatches_to_runner(self) -> None: + with patch( + "mcpbridge_wrapper.__main__.sys.argv", + ["mcpbridge-wrapper", "--doctor", "--web-ui-port", "9191"], + ), patch("mcpbridge_wrapper.doctor.run_doctor", return_value=1) as run_doctor: + result = main() + + assert result == 1 + run_doctor.assert_called_once_with( + web_ui_port=9191, + web_ui_config=None, + ) + + def test_main_doctor_rejects_broker_flags(self, capsys) -> None: + with patch( + "mcpbridge_wrapper.__main__.sys.argv", + ["mcpbridge-wrapper", "--doctor", "--broker"], + ): + result = main() + + assert result == 2 + assert "--doctor cannot be combined with broker mode flags" in capsys.readouterr().err + + def test_main_doctor_rejects_webui_flags(self, capsys) -> None: + with patch( + "mcpbridge_wrapper.__main__.sys.argv", + ["mcpbridge-wrapper", "--doctor", "--web-ui"], + ): + result = main() + + assert result == 2 + assert "--doctor cannot be combined with --web-ui flags" in capsys.readouterr().err + + def test_main_doctor_rejects_interactive_modes(self, capsys) -> None: + with patch( + "mcpbridge_wrapper.__main__.sys.argv", + ["mcpbridge-wrapper", "--doctor", "--tui"], + ): + result = main() + + assert result == 2 + assert ( + "--doctor cannot be combined with --tui or --broker-console" in capsys.readouterr().err + ) + + def test_main_doctor_rejects_bridge_args(self, capsys) -> None: + with patch( + "mcpbridge_wrapper.__main__.sys.argv", + ["mcpbridge-wrapper", "--doctor", "--", "--foo"], + ): + result = main() + + assert result == 2 + assert "--doctor does not accept bridge arguments" in capsys.readouterr().err From aec3ce16fe681d874a26e915c955acfd401f75c3 Mon Sep 17 00:00:00 2001 From: Egor Merkushev Date: Sat, 7 Mar 2026 18:05:17 +0300 Subject: [PATCH 5/7] Archive task P7-T2: Implement a broker doctor command for cross-black-box diagnostics (PASS) --- SPECS/ARCHIVE/INDEX.md | 4 +++- ...command_for_cross-black-box_diagnostics.md | 4 ++++ .../P7-T2_Validation_Report.md | 0 SPECS/INPROGRESS/next.md | 23 +++++++++++-------- SPECS/Workplan.md | 9 ++++---- 5 files changed, 26 insertions(+), 14 deletions(-) rename SPECS/{INPROGRESS => ARCHIVE/P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics}/P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics.md (99%) rename SPECS/{INPROGRESS => ARCHIVE/P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics}/P7-T2_Validation_Report.md (100%) diff --git a/SPECS/ARCHIVE/INDEX.md b/SPECS/ARCHIVE/INDEX.md index fb253e9..3a97a1a 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-T1 review archived) +**Last Updated:** 2026-03-07 (P7-T2 archived) ## Archived Tasks | Task ID | Folder | Archived | Verdict | |---------|--------|----------|---------| +| 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 | | P6-T2 | [P6-T2_Build_a_terminal_frontend_for_broker_daemon_monitoring_and_control/](P6-T2_Build_a_terminal_frontend_for_broker_daemon_monitoring_and_control/) | 2026-03-07 | PASS | @@ -605,3 +606,4 @@ | 2026-03-06 | BUG-T9 | Archived Fix_broker_daemon_not_sending_notifications_initialized_before_tools_list_probe (PASS) | | 2026-03-06 | BUG-T9 | Archived REVIEW_BUG-T9 report | | 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) | diff --git a/SPECS/INPROGRESS/P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics.md b/SPECS/ARCHIVE/P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics/P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics.md similarity index 99% rename from SPECS/INPROGRESS/P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics.md rename to SPECS/ARCHIVE/P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics/P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics.md index 6731427..cdd2537 100644 --- a/SPECS/INPROGRESS/P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics.md +++ b/SPECS/ARCHIVE/P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics/P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics.md @@ -139,3 +139,7 @@ Verification: - Any new remediation text should align with the recommended dedicated-host workflow introduced in `P7-T1`. - Review subject name for this task: `broker_doctor_diagnostics`. + +--- +**Archived:** 2026-03-07 +**Verdict:** PASS diff --git a/SPECS/INPROGRESS/P7-T2_Validation_Report.md b/SPECS/ARCHIVE/P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics/P7-T2_Validation_Report.md similarity index 100% rename from SPECS/INPROGRESS/P7-T2_Validation_Report.md rename to SPECS/ARCHIVE/P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics/P7-T2_Validation_Report.md diff --git a/SPECS/INPROGRESS/next.md b/SPECS/INPROGRESS/next.md index 901c90b..f3d1a3b 100644 --- a/SPECS/INPROGRESS/next.md +++ b/SPECS/INPROGRESS/next.md @@ -1,19 +1,24 @@ -# Next Task: P7-T2 — Implement a broker doctor command for cross-black-box diagnostics +# Next Task: P7-T3 — Auto-recover or guide on dashboard port ownership conflicts **Priority:** P0 **Phase:** Phase 7: Broker UX and Diagnostics **Effort:** 4-6 hours -**Dependencies:** P6-T1 -**Status:** Selected +**Dependencies:** P6-T1, P7-T2 +**Status:** Ready after `P7-T2` CI clears ## Description -Add a `doctor`-style diagnostic command that inspects the full user-visible -chain: Python runtime, local broker files and processes, dashboard endpoint -ownership, and common failure modes such as stale ports, missing dashboard, or -wrong endpoint. The output should help users debug without needing to infer the -internal broker architecture first. +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. + +## Recently Archived + +- `2026-03-07` — `P7-T2` archived with verdict `PASS` ## Next Step -Run the PLAN command to produce the implementation-ready PRD for `P7-T2`. +Wait for the `P7-T2` pull request to clear CI, then run FLOW again to select +and plan `P7-T3`. diff --git a/SPECS/Workplan.md b/SPECS/Workplan.md index 23ede2e..1e7cbd0 100644 --- a/SPECS/Workplan.md +++ b/SPECS/Workplan.md @@ -497,7 +497,8 @@ Add new tasks using the canonical template in [TASK_TEMPLATE.md](TASK_TEMPLATE.m - [ ] `--broker-console` returns exit code `0` on `KeyboardInterrupt` whether it spawns a host or reuses an existing broker-backed dashboard - [ ] Unit tests cover the reuse-existing-dashboard interrupt path -#### ⬜️ P7-T2: Implement a broker doctor command for cross-black-box diagnostics +#### ✅ P7-T2: Implement a broker doctor command for cross-black-box diagnostics +- **Status:** ✅ Completed (2026-03-07) - **Description:** Add a `doctor`-style diagnostic command that inspects the full chain visible to the user: Python package/runtime, local broker files and processes, dashboard endpoint ownership, upstream Xcode bridge state when observable, and common failure modes such as stale ports, missing dashboard, version mismatch, or wrong endpoint. The output should help users debug without needing to understand the internal architecture first. - **Priority:** P0 - **Dependencies:** P6-T1 @@ -507,9 +508,9 @@ Add new tasks using the canonical template in [TASK_TEMPLATE.md](TASK_TEMPLATE.m - new diagnostics module that checks broker PID/socket/version files, HTTP endpoints, occupied ports, and current package/runtime identity - unit/integration tests covering healthy, missing-dashboard, wrong-port, and stale-runtime scenarios - **Acceptance Criteria:** - - [ ] A single command prints a concise diagnosis of broker health and the most likely next action when startup failed - - [ ] The diagnostics distinguish between “broker alive but no dashboard”, “dashboard alive but wrong service”, “port already occupied”, and “broker not running” - - [ ] Output is user-facing and actionable without requiring users to manually run `lsof`, `curl`, or inspect raw log files first + - [x] A single command prints a concise diagnosis of broker health and the most likely next action when startup failed + - [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. From ebf9d6573a65617c34df9821411559b3d912ee24 Mon Sep 17 00:00:00 2001 From: Egor Merkushev Date: Sat, 7 Mar 2026 18:05:58 +0300 Subject: [PATCH 6/7] Review P7-T2: broker doctor diagnostics --- .../REVIEW_broker_doctor_diagnostics.md | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 SPECS/INPROGRESS/REVIEW_broker_doctor_diagnostics.md diff --git a/SPECS/INPROGRESS/REVIEW_broker_doctor_diagnostics.md b/SPECS/INPROGRESS/REVIEW_broker_doctor_diagnostics.md new file mode 100644 index 0000000..03d9a22 --- /dev/null +++ b/SPECS/INPROGRESS/REVIEW_broker_doctor_diagnostics.md @@ -0,0 +1,47 @@ +## REVIEW REPORT — broker_doctor_diagnostics + +**Scope:** origin/main..HEAD +**Files:** 9 + +### Summary Verdict +- [x] Approve +- [ ] Approve with comments +- [ ] Request changes +- [ ] Block + +### Critical Issues + +None. + +### Secondary Issues + +None. + +### Architectural Notes + +- The new `doctor.py` module cleanly centralizes broker diagnostics instead of + burying them in `__main__.py` or the TUI layer, which keeps `P7-T4` and + `P7-T5` unblocked. +- Reusing `build_tui_runtime()` and `BrokerTUIClient` keeps endpoint resolution + aligned with the existing frontend surface and avoids introducing another + dashboard-targeting contract. +- The diagnosis buckets cover the user-visible failure modes called out in the + workplan: stale local runtime, missing dashboard, wrong service, occupied + port, degraded broker-backed state, and healthy runtime. + +### Tests + +- `pytest tests/unit/test_doctor.py tests/unit/test_main_doctor.py` passed +- `python -m mcpbridge_wrapper --doctor` produced a healthy diagnosis against + the current local dedicated-host runtime +- Full quality gates passed: + - `pytest` + - `ruff check src/ tests/` + - `mypy src/` + - `make format-check` + - `pytest --cov=src --cov-report=term` +- Repository coverage remains above threshold at `91.72%` + +### Next Steps + +- No actionable review findings. FOLLOW-UP is skipped for `P7-T2`. From 0d0c153b2419c25192ed504aedf414d20d04f4ad Mon Sep 17 00:00:00 2001 From: Egor Merkushev Date: Sat, 7 Mar 2026 18:06:21 +0300 Subject: [PATCH 7/7] Archive REVIEW_broker_doctor_diagnostics report --- SPECS/ARCHIVE/INDEX.md | 4 +++- .../_Historical}/REVIEW_broker_doctor_diagnostics.md | 0 2 files changed, 3 insertions(+), 1 deletion(-) rename SPECS/{INPROGRESS => ARCHIVE/_Historical}/REVIEW_broker_doctor_diagnostics.md (100%) diff --git a/SPECS/ARCHIVE/INDEX.md b/SPECS/ARCHIVE/INDEX.md index 3a97a1a..379237c 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-T2 archived) +**Last Updated:** 2026-03-07 (P7-T2 review archived) ## Archived Tasks @@ -198,6 +198,7 @@ | File | Description | |------|-------------| +| [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 | | [REVIEW_broker_terminal_frontend.md](_Historical/REVIEW_broker_terminal_frontend.md) | Review report for P6-T2 | @@ -607,3 +608,4 @@ | 2026-03-06 | BUG-T9 | Archived REVIEW_BUG-T9 report | | 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 | diff --git a/SPECS/INPROGRESS/REVIEW_broker_doctor_diagnostics.md b/SPECS/ARCHIVE/_Historical/REVIEW_broker_doctor_diagnostics.md similarity index 100% rename from SPECS/INPROGRESS/REVIEW_broker_doctor_diagnostics.md rename to SPECS/ARCHIVE/_Historical/REVIEW_broker_doctor_diagnostics.md