Skip to content
4 changes: 3 additions & 1 deletion SPECS/ARCHIVE/INDEX.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
# mcpbridge-wrapper Tasks Archive

**Last Updated:** 2026-03-01 (P2-T4 archived)
**Last Updated:** 2026-03-01 (P2-T5 archived)

## Archived Tasks

| Task ID | Folder | Archived | Verdict |
|---------|--------|----------|---------|
| P2-T5 | [P2-T5_Warn_when_broker_lacks_web_ui/](P2-T5_Warn_when_broker_lacks_web_ui/) | 2026-03-01 | PASS |
| P2-T4 | [P2-T4_Surface_broker_unavailability_as_JSONRPC_error/](P2-T4_Surface_broker_unavailability_as_JSONRPC_error/) | 2026-03-01 | PASS |
| P2-T3 | [P2-T3_Fix_double_spawn_race_condition/](P2-T3_Fix_double_spawn_race_condition/) | 2026-03-01 | PASS |
| P2-T1 | [P2-T1_Replace_broker_flags_with_single_broker_flag/](P2-T1_Replace_broker_flags_with_single_broker_flag/) | 2026-03-01 | PASS |
Expand Down Expand Up @@ -175,6 +176,7 @@

| File | Description |
|------|-------------|
| [REVIEW_P2-T5_webui_mismatch_warning.md](_Historical/REVIEW_P2-T5_webui_mismatch_warning.md) | Review report for P2-T5 |
| [REVIEW_P2-T4_broker_unavailable_error.md](_Historical/REVIEW_P2-T4_broker_unavailable_error.md) | Review report for P2-T4 |
| [REVIEW_P2-T3_spawn_lock.md](_Historical/REVIEW_P2-T3_spawn_lock.md) | Review report for P2-T3 |
| [REVIEW_P2-T1_broker_flag.md](_Historical/REVIEW_P2-T1_broker_flag.md) | Review report for P2-T1 |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Validation Report: P2-T5 — Warn when --web-ui requested but running broker lacks it

**Date:** 2026-03-01
**Status:** PASS

## Changes Delivered

| File | Change |
|------|--------|
| `src/mcpbridge_wrapper/broker/proxy.py` | Added `web_ui_port` param and `_new_broker_spawned` flag to `BrokerProxy`; added `_warn_web_ui_mismatch()` helper; added mismatch check call in `run()` after connect; set `_new_broker_spawned=True` in `_spawn_broker_if_needed` before Popen |
| `src/mcpbridge_wrapper/__main__.py` | Pass effective web UI port (`web_ui_port or 8080` when `web_ui_enabled`, else `None`) to `BrokerProxy` |
| `tests/unit/test_broker_proxy.py` | Added `TestBrokerProxyWebUIMismatch` (5 tests) |

## Acceptance Criteria

- [x] When `--web-ui` is passed to proxy but running broker has no web UI, a warning is printed to stderr
- [x] Warning text is actionable (mentions `broker.sock`, `broker.pid`, how to reconnect with `--broker --web-ui`)
- [x] MCP session continues normally despite the warning
- [x] All existing tests pass
- [x] `pytest --cov` coverage ≥ 90% (achieved 91.66%)
- [x] `ruff check src/` passes
- [x] `ruff format --check src/ tests/` passes

## Quality Gates

| Gate | Result |
|------|--------|
| `pytest -q` | 737 passed, 5 skipped |
| `pytest --cov` coverage | 91.66% (≥ 90% required) |
| `ruff check src/` | PASS |
| `ruff format --check src/ tests/` | PASS |

## Implementation Notes

- `_warn_web_ui_mismatch()` is synchronous — it uses `socket.socket` with a 0.5 s timeout
for a TCP probe to `127.0.0.1:{web_ui_port}`. This cannot block the event loop for more than
0.5 s and requires no new dependencies.
- The `_new_broker_spawned` flag prevents false-positive warnings immediately after spawning a
new broker (the HTTP server may not be ready yet). When an existing broker is found (PID file
alive or socket liveness check passes), the flag remains `False` and the probe runs.
- The default web UI port (8080) is used in `__main__.py` when `--web-ui` is passed but
`--web-ui-port` is not explicitly set.
- Warning goes to `sys.stderr` only — the JSON-RPC stream on stdout is not affected.
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
# PRD: P2-T5 — Warn when --web-ui requested but running broker lacks it

## Overview

When the user starts a proxy with `--broker --web-ui` and a broker daemon is already running
without the web UI, the proxy silently connects to the existing daemon and `--web-ui` has no
effect. The user sees no web UI and no explanation. This task adds a stderr warning when the
mismatch is detected.

## Problem Statement

`_spawn_broker_if_needed` has two paths:
1. Existing broker alive → connect (skip spawn). `--web-ui` flag was **not** passed to it at
startup, so it has no web UI.
2. No broker → spawn a new daemon with the `spawn_args` including `--web-ui`.

In path 1, the proxy connects successfully, the MCP session works, but the web dashboard the
user expects is missing with no explanation.

## Proposed Solution

### Detection strategy

After connecting to an existing broker (path 1 above), attempt a TCP probe to
`127.0.0.1:{web_ui_port}` with a 0.5 s timeout. If the probe fails (`ConnectionRefusedError`
or `socket.timeout`), the running broker has no web UI — emit the warning.

If a new broker was just spawned (path 2), skip the probe — the daemon needs time to start its
HTTP server, and the user's intent was correctly expressed in `spawn_args`.

### Warning text

```
Warning: broker is running without --web-ui. Restart the broker to enable the dashboard.
Hint: kill the running broker (rm ~/.mcpbridge_wrapper/broker.sock broker.pid) then reconnect.
```

The warning is printed to **stderr** so it does not corrupt the MCP JSON-RPC stream on stdout.

## Deliverables

| File | Change |
|------|--------|
| `src/mcpbridge_wrapper/broker/proxy.py` | Add `web_ui_port` param; track `_new_broker_spawned`; add `_warn_web_ui_mismatch()` |
| `src/mcpbridge_wrapper/__main__.py` | Pass effective web UI port to `BrokerProxy` |
| `tests/unit/test_broker_proxy.py` | Add `TestBrokerProxyWebUIMismatch` (≥4 tests) |

## Implementation Plan

### 1. `proxy.py`

**`__init__`**: add `web_ui_port: int | None = None` parameter.

**Instance state**: add `self._web_ui_port = web_ui_port` and
`self._new_broker_spawned: bool = False`.

**`_spawn_broker_if_needed`**: set `self._new_broker_spawned = True` immediately before the
`subprocess.Popen` call.

**`run()`**: after successful connect (after the outer try/except block), add:
```python
if self._web_ui_port is not None and not self._new_broker_spawned:
self._warn_web_ui_mismatch()
```

**`_warn_web_ui_mismatch()`** (synchronous helper):
```python
def _warn_web_ui_mismatch(self) -> None:
port = self._web_ui_port
try:
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
s.settimeout(0.5)
s.connect(("127.0.0.1", port))
# Port is accepting — web UI is present; no warning needed.
except OSError:
print(
f"Warning: broker is running without --web-ui on port {port}. "
"Restart the broker to enable the dashboard.\n"
" Hint: kill the running broker "
"(rm ~/.mcpbridge_wrapper/broker.sock ~/.mcpbridge_wrapper/broker.pid) "
"then reconnect with --broker --web-ui.",
file=sys.stderr,
)
```

### 2. `__main__.py`

In the `broker_connect` block, pass effective web UI port to `BrokerProxy`:

```python
_WEB_UI_DEFAULT_PORT = 8080

proxy = BrokerProxy(
broker_config,
auto_spawn=broker_spawn,
connect_timeout=10.0,
spawn_args=_build_broker_spawn_args(...),
web_ui_port=(web_ui_port if web_ui_port is not None else _WEB_UI_DEFAULT_PORT)
if web_ui_enabled
else None,
)
```

### 3. `tests/unit/test_broker_proxy.py`

Add `TestBrokerProxyWebUIMismatch`:
- `test_warning_printed_when_port_refused` — web_ui_port set, existing broker, probe fails → warning in stderr
- `test_no_warning_when_port_listening` — existing broker, probe succeeds → no warning
- `test_no_warning_when_new_broker_spawned` — new spawn, port not listening → no warning (skips probe)
- `test_no_warning_when_web_ui_port_not_set` — web_ui_port=None → no warning

## Acceptance Criteria

- [ ] When `--web-ui` is passed to proxy but running broker has no web UI, a warning is printed to stderr
- [ ] Warning text is actionable (tells user how to restart the broker)
- [ ] MCP session continues normally despite the warning
- [ ] All existing tests pass
- [ ] `pytest --cov` coverage ≥ 90%
- [ ] `ruff check src/` and `ruff format --check src/ tests/` pass

## Dependencies

None.

## Risk

Low — the probe is a non-blocking TCP connect with 0.5 s timeout; it cannot hang the proxy.
The warning is stderr-only; it does not affect the JSON-RPC stream.
83 changes: 83 additions & 0 deletions SPECS/ARCHIVE/_Historical/REVIEW_P2-T5_webui_mismatch_warning.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
## REVIEW REPORT — P2-T5: Web UI mismatch warning

**Scope:** origin/main..HEAD
**Files:** 3 changed (proxy.py, __main__.py, test_broker_proxy.py)
**Date:** 2026-03-01

### Summary Verdict
- [x] Approve with comments
- [ ] Approve
- [ ] Request changes
- [ ] Block

---

### Critical Issues

None.

---

### Secondary Issues

**[Low] TCP probe is synchronous and blocks the event loop for up to 0.5 s**

`_warn_web_ui_mismatch()` calls `socket.connect()` synchronously. While 0.5 s is a short
wall-clock time, it blocks the asyncio event loop for the full timeout duration when the port
is not listening. For a single-proxy use case this is negligible, but it is architecturally
inconsistent with the rest of the proxy code, which is fully async.

**Suggested future fix (optional):** Wrap the probe in `asyncio.wait_for(loop.run_in_executor(...), timeout=0.5)`.
Not a blocker — the current behaviour is functionally correct and the window is bounded.

**[Low] Hardcoded default port 8080 in `__main__.py`**

The default web UI port (8080) is defined inline in `main()` rather than referencing the
canonical default from `WebUIConfig`. If the default ever changes in `WebUIConfig`, the
proxy's mismatch probe would silently use the wrong port.

**Suggested future fix (optional):** Extract `_WEB_UI_DEFAULT_PORT` as a module-level constant
or import it from `webui.config`. Not a blocker — current value matches.

**[Nit] `_new_broker_spawned` is an instance variable set in a helper method**

Setting `self._new_broker_spawned = True` inside `_spawn_broker_if_needed()` creates a hidden
state dependency. The attribute is initialized in `__init__` to `False`, so the default is
safe, but it's not immediately obvious from reading `run()` that the flag is set as a side
effect. A comment in `run()` pointing to the flag's setter is sufficient (already present).

---

### Architectural Notes

- The TCP probe approach is pragmatic: no new infrastructure (no status file, no broker status
endpoint) required. The only assumption is that the web UI is on `127.0.0.1:{port}`, which
matches all current deployment scenarios.
- `_new_broker_spawned` correctly prevents false-positive warnings when the proxy just spawned
a new daemon — the HTTP server may not be ready, but `--web-ui` was already passed in
`spawn_args`.
- The warning is stderr-only; the JSON-RPC stream is unaffected. Session continues normally.

---

### Tests

- Added `TestBrokerProxyWebUIMismatch` (5 tests):
- `test_warning_printed_when_port_refused` — probe fails → warning in stderr
- `test_no_warning_when_port_listening` — probe succeeds → no warning
- `test_no_warning_when_new_broker_spawned` — spawned → probe skipped
- `test_no_warning_when_web_ui_port_not_set` — port=None → no probe, no warning
- `test_warning_is_actionable` — warning text contains `broker.sock` / `Restart`
- Coverage: 91.66% (≥ 90%). ✅
- All 737 tests pass. ✅

---

### Next Steps

- **Optional FU:** Make TCP probe async (`run_in_executor`) to avoid blocking the event loop.
Low priority — 0.5 s is acceptable for the one-time startup path.
- **Optional FU:** Centralise default web UI port constant to avoid divergence with
`WebUIConfig._DEFAULTS["port"]`.

**VERDICT: PASS — no blockers; two low-severity and one nit observation.**
8 changes: 2 additions & 6 deletions SPECS/INPROGRESS/next.md
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
# No Active Task

**Status:** Idle — P2-T4 archived. Select the next task from `SPECS/Workplan.md`.
**Status:** Idle — P2-T5 archived. Select the next task from `SPECS/Workplan.md`.

## Recently Archived

- **P2-T5** — Warn or restart daemon when --web-ui requested but running broker lacks it (2026-03-01, PASS)
- **P2-T4** — Surface broker unavailability as JSON-RPC error instead of silent timeout (2026-03-01, PASS)
- **P2-T3** — Fix double-spawn race condition when MCP client toggles rapidly (2026-03-01, PASS)
- **P2-T1** — Replace --broker-spawn/--broker-connect with single --broker flag (2026-03-01, PASS)
- **P2-T2** — Self-healing stale socket and PID file recovery (2026-03-01, PASS)
- **BUG-T8** — Fix broker proxy bridge exits after first write due to BaseProtocol missing _drain_helper (2026-03-01, PASS)

## Suggested Next Tasks

- **P2-T5** (P2) — Warn or restart daemon when --web-ui requested but running broker lacks it
12 changes: 7 additions & 5 deletions SPECS/Workplan.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,17 +112,19 @@ Add new tasks using the canonical template in [TASK_TEMPLATE.md](TASK_TEMPLATE.m
- [x] Error message includes a human-readable reason (timeout, refused, stale socket)
- [x] Client does not hang indefinitely — error is returned within `connect_timeout` seconds

#### ⬜️ P2-T5: Warn or restart daemon when --web-ui requested but running broker lacks it
#### ✅ P2-T5: Warn or restart daemon when --web-ui requested but running broker lacks it
- **Status:** ✅ Completed (2026-03-01)
- **Description:** When a user configures `--broker-spawn --web-ui` and a broker daemon is already running without the web UI, the proxy connects silently and the `--web-ui` flag has no effect. The user sees 0 web UI and no explanation. Fix by detecting the mismatch: if the proxy is asked for web UI but the running daemon does not expose a web UI port (detectable via a broker status endpoint or absence of HTTP response on the expected port), emit a clear warning to stderr: `"Warning: broker is running without --web-ui. Restart the broker to enable the dashboard."`.
- **Priority:** P2
- **Dependencies:** none
- **Parallelizable:** yes
- **Outputs/Artifacts:**
- `src/mcpbridge_wrapper/broker/proxy.py` — web UI mismatch detection and warning
- `src/mcpbridge_wrapper/broker/proxy.py` — `_warn_web_ui_mismatch()` helper; `web_ui_port` param; `_new_broker_spawned` flag
- `src/mcpbridge_wrapper/__main__.py` — passes effective web UI port to `BrokerProxy`
- **Acceptance Criteria:**
- [ ] When `--web-ui` is passed to proxy but running broker has no web UI, a warning is printed to stderr
- [ ] Warning text is actionable (tells user how to fix it)
- [ ] MCP session continues normally despite the warning
- [x] When `--web-ui` is passed to proxy but running broker has no web UI, a warning is printed to stderr
- [x] Warning text is actionable (tells user how to fix it)
- [x] MCP session continues normally despite the warning

### Bug Fixes

Expand Down
30 changes: 30 additions & 0 deletions src/mcpbridge_wrapper/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,31 @@ def _prepare_webui_runtime(
return config, metrics, audit, is_port_available, run_server, run_server_in_thread


def _effective_web_ui_port(
*,
web_ui_enabled: bool,
web_ui_port: Optional[int],
web_ui_config: Optional[str],
) -> Optional[int]:
"""Return the effective web UI port for the broker mismatch probe.

When ``--web-ui-port`` is explicit, use it directly. Otherwise derive the
port from ``--web-ui-config`` (via WebUIConfig) so that the probe targets
the same port the broker was configured with. Falls back to 8080 if the
webui extras are not installed.
"""
if not web_ui_enabled:
return None
if web_ui_port is not None:
return web_ui_port
try:
from mcpbridge_wrapper.webui.config import WebUIConfig

return WebUIConfig(config_path=web_ui_config).port
except ImportError:
return 8080


def _build_broker_spawn_args(
*,
web_ui_enabled: bool,
Expand Down Expand Up @@ -563,6 +588,11 @@ def main() -> int:
web_ui_config=web_ui_config,
web_ui_restart=web_ui_restart,
),
web_ui_port=_effective_web_ui_port(
web_ui_enabled=web_ui_enabled,
web_ui_port=web_ui_port,
web_ui_config=web_ui_config,
),
)
try:
asyncio.run(proxy.run())
Expand Down
Loading