diff --git a/SPECS/ARCHIVE/FU-P7-T3-1_Prioritize_foreign_port-owner_guidance_in_mixed_broker-dashboard_conflicts/FU-P7-T3-1_Prioritize_foreign_port-owner_guidance_in_mixed_broker-dashboard_conflicts.md b/SPECS/ARCHIVE/FU-P7-T3-1_Prioritize_foreign_port-owner_guidance_in_mixed_broker-dashboard_conflicts/FU-P7-T3-1_Prioritize_foreign_port-owner_guidance_in_mixed_broker-dashboard_conflicts.md new file mode 100644 index 0000000..f1de871 --- /dev/null +++ b/SPECS/ARCHIVE/FU-P7-T3-1_Prioritize_foreign_port-owner_guidance_in_mixed_broker-dashboard_conflicts/FU-P7-T3-1_Prioritize_foreign_port-owner_guidance_in_mixed_broker-dashboard_conflicts.md @@ -0,0 +1,127 @@ +# FU-P7-T3-1 — Prioritize foreign port-owner guidance in mixed broker/dashboard conflicts + +## Objective Summary + +`P7-T3` removed the silent “broker alive, dashboard absent” partial state, but +its review found one remaining mixed-state failure mode: when a broker PID is +still live and the requested dashboard port is simultaneously occupied by a +foreign listener, startup guidance prioritizes the broker reset path and can +hide the actual port owner. That can send users into a reset loop while the +real blocker, the unrelated listener on the port, remains untouched. + +This follow-up should make mixed-state diagnostics explicit across both startup +and `--doctor`. The goal is not to invent a new recovery workflow. Instead, +the code should surface the foreign port owner first or name both blockers in +one clear remediation path, while preserving the existing single-listener and +broker-without-dashboard behavior for non-mixed states. + +## Deliverables + +- Update `src/mcpbridge_wrapper/__main__.py` so mixed-state startup failures + can report both a live broker PID and a foreign dashboard-port listener + instead of hiding the listener behind broker-reset guidance. +- Update `src/mcpbridge_wrapper/doctor.py` so mixed broker/listener conflicts + classify as a port-ownership issue rather than the generic + `broker-without-dashboard` diagnosis. +- Add regression coverage in `tests/unit/test_main.py` and + `tests/unit/test_doctor.py` for the mixed-state conflict path. +- Produce `SPECS/INPROGRESS/FU-P7-T3-1_Validation_Report.md` with required + quality-gate evidence. + +## Success Criteria + +- `--broker-console` and `--broker-daemon --web-ui` mention the foreign + dashboard-port listener or both blockers when a live broker PID and foreign + listener coexist. +- `--doctor` no longer hides the foreign listener behind a generic + broker-without-dashboard diagnosis in the same mixed state. +- Regression tests pin the mixed-state ordering so future changes cannot revert + to broker-only guidance. + +## Test-First Plan + +1. Add startup tests that model the mixed state for both `_run_broker_console()` + and `main()`’s `--broker-daemon --web-ui` path, asserting the foreign port + owner is surfaced alongside the live broker PID. +2. Add a doctor-classification test for a live local broker plus foreign + listener on the configured dashboard port, asserting the report stays in the + port-occupied family instead of `broker-without-dashboard`. +3. Implement the smallest production changes needed to collect both blockers + before choosing the user-facing guidance path. +4. Run the required quality gates: `pytest`, `ruff check src/`, `mypy src/`, + and `pytest --cov`. + +## Execution Plan + +### Phase 1: Pin the mixed-state startup contract + +Inputs: +- `src/mcpbridge_wrapper/__main__.py` +- existing `P7-T3` startup tests in `tests/unit/test_main.py` + +Outputs: +- regression tests for mixed-state broker PID + foreign listener conflicts +- a precise expected stderr contract for broker-console and broker-daemon flows + +Verification: +- the new startup tests fail on the current ordering that hides the foreign + listener + +### Phase 2: Align startup guidance + +Inputs: +- `_run_broker_console()` +- `_report_requested_dashboard_unavailable()` +- broker-daemon web-ui startup branch in `main()` + +Outputs: +- startup logic that can see both `running_broker_pid` and `listener_pids` +- one explicit remediation path that surfaces the foreign listener without + dropping broker-state context + +Verification: +- startup messaging for single-blocker states remains unchanged +- mixed-state messaging mentions the foreign listener or both blockers + +### Phase 3: Align doctor classification and validate + +Inputs: +- `src/mcpbridge_wrapper/doctor.py` +- doctor classification tests +- full repo quality gates + +Outputs: +- mixed-state doctor report that stays actionable and consistent with startup +- validation report with targeted and full quality-gate evidence + +Verification: +- `--doctor` and startup both direct the user toward resolving the foreign port + conflict before or alongside broker reset +- coverage remains at or above the repository threshold + +## Acceptance Tests + +- `pytest tests/unit/test_main.py` +- `pytest tests/unit/test_doctor.py` +- `pytest` +- `ruff check src/` +- `mypy src/` +- `pytest --cov` + +## Decision Points + +- Prefer a combined-message approach when both blockers are real; users should + not lose visibility into the live broker PID just because the foreign + listener is now prioritized. +- Keep the public recovery commands aligned with `P7-T3` and `P7-T2` rather + than inventing a new remediation surface for this follow-up. + +## Notes + +- No documentation changes are expected unless the implementation forces + observable command/help text changes beyond stderr diagnostics. +- Review subject name for this task: `mixed_dashboard_conflict_guidance`. + +--- +**Archived:** 2026-03-07 +**Verdict:** PASS diff --git a/SPECS/ARCHIVE/FU-P7-T3-1_Prioritize_foreign_port-owner_guidance_in_mixed_broker-dashboard_conflicts/FU-P7-T3-1_Validation_Report.md b/SPECS/ARCHIVE/FU-P7-T3-1_Prioritize_foreign_port-owner_guidance_in_mixed_broker-dashboard_conflicts/FU-P7-T3-1_Validation_Report.md new file mode 100644 index 0000000..2fe25d4 --- /dev/null +++ b/SPECS/ARCHIVE/FU-P7-T3-1_Prioritize_foreign_port-owner_guidance_in_mixed_broker-dashboard_conflicts/FU-P7-T3-1_Validation_Report.md @@ -0,0 +1,99 @@ +# FU-P7-T3-1 Validation Report + +**Task:** FU-P7-T3-1 — Prioritize foreign port-owner guidance in mixed broker/dashboard conflicts +**Date:** 2026-03-07 +**Verdict:** PASS + +## Summary + +Implemented the `FU-P7-T3-1` follow-up by: + +- updating startup guidance in `src/mcpbridge_wrapper/__main__.py` so mixed + broker/dashboard conflicts can surface both a live broker PID and a foreign + listener on the requested dashboard port +- fixing `_run_broker_console()` so it inspects the configured dashboard port + before returning early on a running broker PID, allowing mixed-state + reporting instead of broker-only guidance +- aligning `src/mcpbridge_wrapper/doctor.py` so mixed broker-plus-listener + states classify as a port-ownership issue rather than being hidden behind the + generic `broker-without-dashboard` diagnosis +- adding regression coverage for broker-console, broker-daemon `--web-ui`, and + doctor mixed-state flows + +## Files Validated + +- `src/mcpbridge_wrapper/__main__.py` +- `src/mcpbridge_wrapper/doctor.py` +- `tests/unit/test_main.py` +- `tests/unit/test_doctor.py` + +## Targeted Verification + +```bash +pytest tests/unit/test_main.py tests/unit/test_doctor.py -k 'mixed_state_mentions_foreign_listener or mixed_broker_and_foreign_listener_prefers_port_conflict' +``` + +- Result: `3 passed` +- Observed outcome: startup and doctor now keep the foreign port owner visible + in mixed broker/listener conflicts + +```bash +pytest tests/unit/test_main.py tests/unit/test_doctor.py +``` + +- Result: `119 passed` + +## Required Quality Gates + +```bash +pytest +``` + +- Result: `891 passed, 5 skipped in 8.14s` + +```bash +ruff check src/ +``` + +- Result: `All checks passed!` + +```bash +mypy src/ +``` + +- Result: `Success: no issues found in 20 source files` + +```bash +pytest --cov +``` + +- Result: `891 passed, 5 skipped in 9.16s` +- Coverage: `91.64%` + +## Acceptance Criteria Evidence + +- [x] `--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. + - Evidence: `tests/unit/test_main.py::TestBrokerConsoleHelpers::test_run_broker_console_mixed_state_mentions_foreign_listener` + and + `tests/unit/test_main.py::TestMainBrokerWebUIFlowCoverage::test_main_broker_daemon_webui_mixed_state_mentions_foreign_listener` + both passed. +- [x] `--doctor` does not hide the foreign listener behind a generic + broker-without-dashboard diagnosis in the same mixed state. + - Evidence: + `tests/unit/test_doctor.py::TestClassifyDoctorReport::test_classify_mixed_broker_and_foreign_listener_prefers_port_conflict` + passed with `report.code == "port-occupied"`. +- [x] Regression tests cover the mixed-state conflict and prevent reordering + back to broker-only guidance. + - Evidence: new mixed-state tests were added in `tests/unit/test_main.py` and + `tests/unit/test_doctor.py`, and the targeted plus full affected-module + suites passed. + +## Notes + +- The mixed-state fix keeps existing single-blocker behavior unchanged: plain + broker-without-dashboard and plain foreign-listener paths still use their + original guidance. +- Existing `websockets` / `uvicorn` deprecation warnings remain in the suite + and are unrelated to this task. diff --git a/SPECS/ARCHIVE/INDEX.md b/SPECS/ARCHIVE/INDEX.md index 4e34d2f..c7b59b0 100644 --- a/SPECS/ARCHIVE/INDEX.md +++ b/SPECS/ARCHIVE/INDEX.md @@ -1,11 +1,12 @@ # mcpbridge-wrapper Tasks Archive -**Last Updated:** 2026-03-07 (FU-P7-T1-1 review archived) +**Last Updated:** 2026-03-07 ## Archived Tasks | Task ID | Folder | Archived | Verdict | |---------|--------|----------|---------| +| FU-P7-T3-1 | [FU-P7-T3-1_Prioritize_foreign_port-owner_guidance_in_mixed_broker-dashboard_conflicts/](FU-P7-T3-1_Prioritize_foreign_port-owner_guidance_in_mixed_broker-dashboard_conflicts/) | 2026-03-07 | PASS | | FU-P7-T1-1 | [FU-P7-T1-1_Normalize_KeyboardInterrupt_handling_when_broker-console_reuses_an_existing_host/](FU-P7-T1-1_Normalize_KeyboardInterrupt_handling_when_broker-console_reuses_an_existing_host/) | 2026-03-07 | PASS | | 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 | @@ -200,6 +201,7 @@ | File | Description | |------|-------------| +| [REVIEW_mixed_dashboard_conflict_guidance.md](_Historical/REVIEW_mixed_dashboard_conflict_guidance.md) | Review report for FU-P7-T3-1 | | [REVIEW_broker_console_keyboardinterrupt_reuse.md](_Historical/REVIEW_broker_console_keyboardinterrupt_reuse.md) | Review report for FU-P7-T1-1 | | [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 | @@ -343,6 +345,8 @@ | Date | Task ID | Action | |------|---------|--------| +| 2026-03-07 | FU-P7-T3-1 | Archived REVIEW_mixed_dashboard_conflict_guidance report | +| 2026-03-07 | FU-P7-T3-1 | Archived Prioritize_foreign_port-owner_guidance_in_mixed_broker-dashboard_conflicts (PASS) | | 2026-03-07 | FU-P7-T1-1 | Archived REVIEW_broker_console_keyboardinterrupt_reuse report | | 2026-03-07 | FU-P7-T1-1 | Archived Normalize_KeyboardInterrupt_handling_when_broker-console_reuses_an_existing_host (PASS) | | 2026-03-07 | P7-T1 | Archived REVIEW_broker_console_startup report | diff --git a/SPECS/ARCHIVE/_Historical/REVIEW_mixed_dashboard_conflict_guidance.md b/SPECS/ARCHIVE/_Historical/REVIEW_mixed_dashboard_conflict_guidance.md new file mode 100644 index 0000000..2efbba8 --- /dev/null +++ b/SPECS/ARCHIVE/_Historical/REVIEW_mixed_dashboard_conflict_guidance.md @@ -0,0 +1,61 @@ +## REVIEW REPORT — mixed_dashboard_conflict_guidance + +**Scope:** origin/main..HEAD +**Files:** 9 + +### Summary Verdict +- [ ] Approve +- [x] Approve with comments +- [ ] Request changes +- [ ] Block + +### Critical Issues + +- [High] `src/mcpbridge_wrapper/__main__.py:613-621`, + `src/mcpbridge_wrapper/__main__.py:761-779`, + `src/mcpbridge_wrapper/doctor.py:437-465`: + the new mixed-state branch treats any listener on the configured dashboard + port as a foreign port conflict whenever a broker PID is also live. That + includes the broker daemon’s own listener if it is still bound to the port + while `/api/control` or `/api/broker/status` is degraded. In that state, the + new guidance can incorrectly tell users to stop the “existing listener” or + use `--web-ui-restart`, even though the listener may be the same broker + process named by the PID file. The fix should distinguish foreign listener + PIDs from the broker’s own PID before taking the mixed-state port-conflict + path. + +### Secondary Issues + +- [Medium] `tests/unit/test_main.py:1788-1824`, + `tests/unit/test_main.py:2276-2329`, + `tests/unit/test_doctor.py:313-326`: + the new regression tests only cover the foreign-listener case. There is no + test where `listener_pids` equals the broker PID (or contains the local + doctor PID), so the self-listener misclassification above would currently + pass unnoticed. Add same-PID coverage alongside the existing foreign-listener + assertions. + +### Architectural Notes + +- The task correctly fixed the user-visible blind spot that motivated + `FU-P7-T3-1`: foreign listeners are now surfaced in the mixed-state path + instead of being hidden behind broker-reset guidance. +- The remaining issue is about PID ownership precision, not about whether mixed + state should be surfaced at all. The next iteration should refine the + classifier to separate foreign listeners from broker-owned listeners. + +### Tests + +- `pytest tests/unit/test_main.py tests/unit/test_doctor.py -k 'mixed_state_mentions_foreign_listener or mixed_broker_and_foreign_listener_prefers_port_conflict'` passed (`3 passed`) +- `pytest tests/unit/test_main.py tests/unit/test_doctor.py` passed (`119 passed`) +- `pytest` passed (`891 passed, 5 skipped`) +- `ruff check src/` passed +- `mypy src/` passed +- `pytest --cov` passed with `91.64%` coverage + +### Next Steps + +- Add a focused follow-up task to distinguish foreign listener PIDs from the + broker’s own PID in startup and doctor mixed-state guidance. +- Add regression coverage for the broker-owned-listener case so the mixed-state + fix does not regress into self-conflict messaging. diff --git a/SPECS/INPROGRESS/next.md b/SPECS/INPROGRESS/next.md index e2ffa21..be61ab0 100644 --- a/SPECS/INPROGRESS/next.md +++ b/SPECS/INPROGRESS/next.md @@ -1,26 +1,26 @@ -# Next Task: FU-P7-T3-1 — Prioritize foreign port-owner guidance in mixed broker/dashboard conflicts +# Next Task: FU-P7-T3-2 — Exclude broker-owned dashboard listeners from foreign port-conflict guidance **Priority:** P1 **Phase:** Phase 7: Broker UX and Diagnostics **Effort:** 2-3 hours -**Dependencies:** P7-T3 +**Dependencies:** FU-P7-T3-1 **Status:** Ready ## 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. +Refine the mixed broker/dashboard conflict classifier so it distinguishes the +broker daemon's own dashboard listener from a foreign process on the same port. +When degraded probes occur against a broker-owned listener, startup and +diagnostics should keep users on broker-health guidance instead of reporting a +foreign port owner. ## Recently Archived +- `2026-03-07` — `FU-P7-T3-1` archived with verdict `PASS` - `2026-03-07` — `FU-P7-T1-1` archived with verdict `PASS` - `2026-03-07` — `P7-T3` archived with verdict `PASS` -- `2026-03-07` — `P7-T2` archived with verdict `PASS` ## Next Step -Create the `FU-P7-T3-1` PRD in `SPECS/INPROGRESS/`, then implement and -validate the mixed-state dashboard conflict guidance updates. +Create the `FU-P7-T3-2` PRD in `SPECS/INPROGRESS/`, then implement and validate +the broker-owned-listener exclusion in startup and doctor mixed-state guidance. diff --git a/SPECS/Workplan.md b/SPECS/Workplan.md index f7945f4..8ab3e2e 100644 --- a/SPECS/Workplan.md +++ b/SPECS/Workplan.md @@ -528,7 +528,8 @@ 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 +#### ✅ FU-P7-T3-1: Prioritize foreign port-owner guidance in mixed broker/dashboard conflicts +- **Status:** ✅ Completed (2026-03-07) - **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 @@ -538,9 +539,23 @@ Add new tasks using the canonical template in [TASK_TEMPLATE.md](TASK_TEMPLATE.m - 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 + - [x] `--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 + - [x] `--doctor` does not hide the foreign listener behind a generic broker-without-dashboard diagnosis in the same mixed state + - [x] Regression tests cover the mixed-state conflict and prevent reordering back to broker-only guidance + +#### ⬜️ FU-P7-T3-2: Exclude broker-owned dashboard listeners from foreign port-conflict guidance +- **Description:** Refine the mixed broker/dashboard conflict classifier so it distinguishes the broker daemon's own dashboard listener from a foreign process on the same port. When degraded probes occur against a broker-owned listener, startup and diagnostics should keep users on broker-health guidance instead of reporting a foreign port owner. +- **Priority:** P1 +- **Dependencies:** FU-P7-T3-1 +- **Parallelizable:** yes +- **Outputs/Artifacts:** + - `src/mcpbridge_wrapper/__main__.py` mixed-state startup guidance that filters broker-owned listener PIDs out of foreign-conflict messaging + - `src/mcpbridge_wrapper/doctor.py` mixed-state diagnostic guidance with the same broker-owned listener exclusion + - `tests/unit/test_main.py` and `tests/unit/test_doctor.py` regression coverage for foreign-listener and broker-owned-listener mixed states +- **Acceptance Criteria:** + - [ ] Mixed-state foreign-port guidance triggers only when the dashboard listener PID differs from the running broker PID + - [ ] Broker-owned listeners with degraded dashboard probes do not tell users to stop an "existing listener" or use restart guidance meant for foreign ownership + - [ ] Regression tests cover both foreign-listener and broker-owned-listener mixed states in startup and doctor paths #### ⬜️ 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. diff --git a/src/mcpbridge_wrapper/__main__.py b/src/mcpbridge_wrapper/__main__.py index b6c20ce..a61e229 100644 --- a/src/mcpbridge_wrapper/__main__.py +++ b/src/mcpbridge_wrapper/__main__.py @@ -610,7 +610,18 @@ def _report_requested_dashboard_unavailable( listener_pids: Set[int], ) -> int: """Print one explicit remediation path for an unusable requested dashboard.""" - if running_broker_pid is not None: + if port is not None and listener_pids and running_broker_pid is not None: + print( + "Error: Broker daemon is already running " + f"(PID {running_broker_pid}), but Web UI port {port} is already occupied by " + f"{_format_listener_pid_summary(listener_pids)}, so no broker-backed " + f"dashboard is reachable at {runtime.base_url}. Stop the existing listener or " + f"retry startup with {_broker_console_restart_command()}. If the port " + f"becomes free and the dashboard is still unavailable, reset the dedicated " + f"host with {_broker_console_reset_command()}.", + file=sys.stderr, + ) + elif 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 " @@ -748,31 +759,33 @@ def _run_console_tui() -> int: if ready: return _run_console_tui() + effective_port = _effective_web_ui_port( + web_ui_enabled=True, + web_ui_port=web_ui_port, + web_ui_config=web_ui_config, + ) + listener_pids: Set[int] = set() + if effective_port is not None and not web_ui_restart: + listener_pids = _find_listener_pids_for_port(effective_port) + running_pid = _read_running_broker_pid() if running_pid is not None: return _report_requested_dashboard_unavailable( runtime=runtime, - port=None, + port=effective_port, probe_error=error, running_broker_pid=running_pid, - listener_pids=set(), + listener_pids=listener_pids, ) - effective_port = _effective_web_ui_port( - web_ui_enabled=True, - web_ui_port=web_ui_port, - web_ui_config=web_ui_config, - ) - if effective_port is not None and not web_ui_restart: - listener_pids = _find_listener_pids_for_port(effective_port) - if listener_pids: - return _report_requested_dashboard_unavailable( - runtime=runtime, - port=effective_port, - probe_error=error, - running_broker_pid=None, - listener_pids=listener_pids, - ) + if listener_pids: + return _report_requested_dashboard_unavailable( + runtime=runtime, + port=effective_port, + probe_error=error, + running_broker_pid=None, + listener_pids=listener_pids, + ) child_process = _spawn_broker_console_host( web_ui_port=web_ui_port, diff --git a/src/mcpbridge_wrapper/doctor.py b/src/mcpbridge_wrapper/doctor.py index 03c3b62..d4b18c4 100644 --- a/src/mcpbridge_wrapper/doctor.py +++ b/src/mcpbridge_wrapper/doctor.py @@ -434,6 +434,37 @@ def classify_doctor_report( evidence_lines=evidence_lines, ) + if local.pid_running and dashboard.listener_pids: + evidence_lines.append( + "A listener already owns the configured dashboard port while the local broker PID " + "is still running." + ) + if dashboard.health_error: + evidence_lines.append(dashboard.health_error) + 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 while the " + "broker daemon is still running." + ), + next_action=( + "Stop the existing listener or retry startup with " + "`mcpbridge-wrapper --broker-console --web-ui-restart`. " + "If the port becomes free and the dashboard is still unavailable, 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, + ) + 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 " diff --git a/tests/unit/test_doctor.py b/tests/unit/test_doctor.py index 6ae85cb..f097868 100644 --- a/tests/unit/test_doctor.py +++ b/tests/unit/test_doctor.py @@ -313,6 +313,23 @@ def test_classify_broker_running_without_dashboard(self) -> None: "mcpbridge-wrapper --broker-console`." ) + def test_classify_mixed_broker_and_foreign_listener_prefers_port_conflict(self) -> None: + dashboard = _dashboard( + listener_pids=[777], + listener_commands={777: "node /tmp/other-service.js"}, + health_ok=False, + backend_error=None, + ) + + report = classify_doctor_report(_runtime(), _local(), 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 + assert any("Daemon PID: 100 (running)" in line for line in report.local_state_lines) + assert any("777" in line for line in report.dashboard_lines) + def test_classify_wrong_service_on_dashboard_port(self) -> None: dashboard = _dashboard( health_ok=True, diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index 9fb123b..1ff2984 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -1788,6 +1788,40 @@ def test_run_broker_console_rejects_stale_running_broker(self, capsys): assert "--broker-console" in err spawn_host.assert_not_called() + def test_run_broker_console_mixed_state_mentions_foreign_listener(self, capsys): + from mcpbridge_wrapper.__main__ import _run_broker_console + + runtime = SimpleNamespace(base_url="http://127.0.0.1:8080", log_path="/tmp/broker.log") + 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=4242, + ), patch( + "mcpbridge_wrapper.__main__._effective_web_ui_port", + return_value=8080, + ), patch( + "mcpbridge_wrapper.__main__._find_listener_pids_for_port", + return_value={999}, + ), patch("mcpbridge_wrapper.__main__._spawn_broker_console_host") as spawn_host: + result = _run_broker_console( + web_ui_port=8080, + web_ui_config=None, + web_ui_restart=False, + ) + + assert result == 1 + err = capsys.readouterr().err + assert "Broker daemon is already running (PID 4242)" in err + assert "Web UI port 8080 is already occupied" in err + assert "listener PID 999" in err + assert "--web-ui-restart" in err + spawn_host.assert_not_called() + def test_run_broker_console_rejects_foreign_dashboard_listener(self, capsys): from mcpbridge_wrapper.__main__ import _run_broker_console @@ -2242,6 +2276,58 @@ def test_main_broker_daemon_webui_fails_fast_when_broker_lacks_dashboard(self, c mock_daemon_cls.assert_not_called() audit.close.assert_called_once_with() + def test_main_broker_daemon_webui_mixed_state_mentions_foreign_listener(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=(False, "GET /api/broker/status failed: Not Found"), + ), patch( + "mcpbridge_wrapper.__main__._read_running_broker_pid", + return_value=4242, + ), 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" + ) as mock_daemon_cls: + mock_cfg_cls.default.return_value = broker_cfg + result = main() + + assert result == 1 + err = capsys.readouterr().err + assert "Broker daemon is already running (PID 4242)" in err + assert "Web UI port 8080 is already occupied" in err + assert "listener PID 999" in err + assert "--web-ui-restart" 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)