Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
# PRD: FU-P12-T1-3 — Show multi-client widgets in Web UI instead of single overwritten active client

**Created:** 2026-02-18
**Priority:** P2
**Branch:** `codex/feature/FU-P12-T1-3-multi-client-widgets`
**Status:** PLAN

---

## 1. Problem Statement

The dashboard currently surfaces one `Active Client` value that is overwritten by
whichever `initialize` handshake happened most recently. This hides concurrent
or recently active clients and makes client attribution harder when multiple MCP
clients are used.

---

## 2. Scope

### In Scope
- Extend metrics summaries to include per-client aggregates (name/version,
last seen, and initialize count).
- Keep existing `client_name`/`client_version` fields for compatibility.
- Update dashboard UI/JS to render one card per detected client.
- Add/adjust unit tests for metrics, shared metrics, and server API behavior.

### Out of Scope
- Historical backfill from old logs.
- Authentication or transport changes.
- Session timeline redesign.

---

## 3. Deliverables

1. Backend metrics data model
- `src/mcpbridge_wrapper/webui/metrics.py`
- `src/mcpbridge_wrapper/webui/shared_metrics.py`
- Summary payload includes `clients` array for dashboard consumption.

2. Web UI rendering updates
- `src/mcpbridge_wrapper/webui/static/index.html`
- `src/mcpbridge_wrapper/webui/static/dashboard.js`
- `src/mcpbridge_wrapper/webui/static/dashboard.css`

3. Test coverage
- `tests/unit/webui/test_metrics.py`
- `tests/unit/webui/test_shared_metrics.py`
- `tests/unit/webui/test_server.py`

4. Validation artifact
- `SPECS/INPROGRESS/FU-P12-T1-3_Validation_Report.md`

---

## 4. Acceptance Criteria

- [ ] Dashboard shows multiple clients simultaneously when more than one client connects.
- [ ] Existing single-client behavior remains correct when only one client is present.
- [ ] Client widgets update in real time with the same refresh cadence as other KPIs.
- [ ] `pytest` passes.
- [ ] `ruff check src/` passes.
- [ ] `mypy src/` passes.
- [ ] `pytest --cov` reports coverage >= 90%.

---

## 5. Dependencies

- P12-T1 ✅

---

## 6. Risks and Mitigations

- **Risk:** Shared SQLite schema migration could break older DB files.
- **Mitigation:** Use additive table creation (`CREATE TABLE IF NOT EXISTS`) and
non-destructive upserts.
- **Risk:** UI changes regress KPI rendering.
- **Mitigation:** Keep existing KPI IDs used by JS and add focused rendering
helpers for new client cards.

---

## 7. Validation Plan

1. Add `clients` summary support in in-memory and shared metrics collectors.
2. Render per-client cards in dashboard from `summary.clients`.
3. Add tests for multi-client summary/API behavior.
4. Run required quality gates and document results.

---

---
**Archived:** 2026-02-18
**Verdict:** PASS
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Validation Report — FU-P12-T1-3

**Task:** FU-P12-T1-3 — Show multi-client widgets in Web UI instead of single overwritten active client
**Date:** 2026-02-18
**Verdict:** PASS

## Scope

- Added multi-client summary support in in-memory and shared SQLite metrics.
- Added dashboard rendering for one widget per detected client.
- Preserved existing `client_name` / `client_version` summary compatibility.
- Added unit coverage for multi-client summary behavior.

## Files Changed

- `src/mcpbridge_wrapper/webui/metrics.py`
- `src/mcpbridge_wrapper/webui/shared_metrics.py`
- `src/mcpbridge_wrapper/webui/static/index.html`
- `src/mcpbridge_wrapper/webui/static/dashboard.js`
- `src/mcpbridge_wrapper/webui/static/dashboard.css`
- `tests/unit/webui/test_metrics.py`
- `tests/unit/webui/test_shared_metrics.py`
- `tests/unit/webui/test_server.py`

## Required Quality Gates

- `pytest`
- Result: **PASS** (`585 passed, 5 skipped, 2 warnings`)
- `ruff check src/`
- Result: **PASS** (`All checks passed!`)
- `mypy src/`
- Result: **PASS** (`Success: no issues found in 18 source files`)
- `pytest --cov`
- Result: **PASS** (`585 passed, 5 skipped, 2 warnings`; total coverage **92.18%**, threshold 90%)

## Acceptance Criteria Status

- [x] Dashboard shows multiple clients simultaneously when more than one client connects.
- [x] Existing single-client behavior remains correct when only one client is present.
- [x] Client widgets update in real time with the same refresh cadence as other KPIs.
- [x] `pytest` suite remains green.

## Notes

- Existing third-party deprecation warnings from `websockets` / `uvicorn` were
observed during tests and are unrelated to this task.
6 changes: 5 additions & 1 deletion SPECS/ARCHIVE/INDEX.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# mcpbridge-wrapper Tasks Archive

**Last Updated:** 2026-02-18 (REVIEW_fu_p12_t1_2_stdin_capture_comment_archived)
**Last Updated:** 2026-02-18 (REVIEW_fu_p12_t1_3_multi_client_widgets_archived)

## Archived Tasks

Expand Down Expand Up @@ -117,6 +117,7 @@
| FU-BUG-T7-1 | [FU-BUG-T7-1_Cap_pending_methods_map_to_guard_unbounded_growth/](FU-BUG-T7-1_Cap_pending_methods_map_to_guard_unbounded_growth/) | 2026-02-18 | PASS |
| FU-P12-T1-1 | [FU-P12-T1-1_Remove_or_document_MCPInitializeParams_in_schemas/](FU-P12-T1-1_Remove_or_document_MCPInitializeParams_in_schemas/) | 2026-02-18 | PASS |
| FU-P12-T1-2 | [FU-P12-T1-2_Add_code_comment_clarifying_stdin-only_client_capture_in_on_request/](FU-P12-T1-2_Add_code_comment_clarifying_stdin-only_client_capture_in_on_request/) | 2026-02-18 | PASS |
| FU-P12-T1-3 | [FU-P12-T1-3_Show_multi-client_widgets_in_Web_UI_instead_of_single_overwritten_active_client/](FU-P12-T1-3_Show_multi-client_widgets_in_Web_UI_instead_of_single_overwritten_active_client/) | 2026-02-18 | PASS |

## Historical Artifacts

Expand Down Expand Up @@ -197,6 +198,7 @@
| [REVIEW_fu_bug_t7_1_pending_methods_cap.md](_Historical/REVIEW_fu_bug_t7_1_pending_methods_cap.md) | Review report for FU-BUG-T7-1 |
| [REVIEW_FU-P12-T1-1_mcpinitializeparams.md](_Historical/REVIEW_FU-P12-T1-1_mcpinitializeparams.md) | Review report for FU-P12-T1-1 |
| [REVIEW_FU-P12-T1-2_stdin_capture_comment.md](_Historical/REVIEW_FU-P12-T1-2_stdin_capture_comment.md) | Review report for FU-P12-T1-2 |
| [REVIEW_FU-P12-T1-3_multi_client_widgets.md](_Historical/REVIEW_FU-P12-T1-3_multi_client_widgets.md) | Review report for FU-P12-T1-3 |

## Archive Log

Expand Down Expand Up @@ -348,3 +350,5 @@
| 2026-02-18 | FU-P12-T1-1 | Archived REVIEW_FU-P12-T1-1_mcpinitializeparams report |
| 2026-02-18 | FU-P12-T1-2 | Archived Add_code_comment_clarifying_stdin-only_client_capture_in_on_request (PASS) |
| 2026-02-18 | FU-P12-T1-2 | Archived REVIEW_FU-P12-T1-2_stdin_capture_comment report |
| 2026-02-18 | FU-P12-T1-3 | Archived Show_multi-client_widgets_in_Web_UI_instead_of_single_overwritten_active_client (PASS) |
| 2026-02-18 | FU-P12-T1-3 | Archived REVIEW_FU-P12-T1-3_multi_client_widgets report |
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
## REVIEW REPORT — FU-P12-T1-3 multi-client widgets

**Scope:** origin/main..HEAD
**Files:** 13

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

### Critical Issues
- None.

### Secondary Issues
- None.

### Architectural Notes
- The summary contract remains backward compatible (`client_name`/`client_version`
preserved) while adding a `clients` array for richer UI rendering.
- Shared-metrics changes use additive schema evolution (`client_identities` table)
and maintain existing `client_info` behavior.
- Dashboard now presents per-client cards and updates through the same websocket
and polling paths used by existing KPIs.

### Tests
- Full quality gates were executed during EXECUTE:
- `pytest` (585 passed, 5 skipped)
- `ruff check src/` (pass)
- `mypy src/` (pass)
- `pytest --cov` (92.18%, threshold 90%)

### Next Steps
- No actionable findings.
- FOLLOW-UP step is skipped per FLOW rules.
47 changes: 47 additions & 0 deletions SPECS/INPROGRESS/REVIEW_FU-P12-T1-3_multi_client_widgets_v2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
## REVIEW REPORT — FU-P12-T1-3 multi-client widgets (v2)

**Scope:** origin/main..HEAD
**Files:** 14 (8 implementation/test, 6 workflow artifacts)

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

### Critical Issues

- [High] **Unbounded `_clients` dict in `MetricsCollector` (metrics.py:83,103-113).**
The in-memory `_clients` dict grows without limit — every unique `(name, version)` pair adds an entry that is never evicted. In the `SharedMetricsStore` the same is true: the `client_identities` table has no pruning. In practice the cardinality is very low (handful of MCP clients), so this is unlikely to cause real problems, but it is inconsistent with the project's existing pattern of capping unbounded maps (see FU-BUG-T7-1 `pending_methods` cap).
**Suggestion:** Add a soft cap (e.g. 50 entries, evict oldest by `last_seen`) to `_clients` in `MetricsCollector`, and consider a `WHERE last_seen > ?` pruning clause for `client_identities` on write.

### Secondary Issues

- [Medium] **`innerHTML` string-building in `renderClientWidgets` (dashboard.js:225-235).**
The `name` and `version` fields are escaped via `escapeHtml`, which is correct. However `count` (an integer) and `lastSeen` (returned from `formatRelativeAge`) are interpolated without escaping. `count` is always a number so this is safe, but `lastSeen` passes through `escapeHtml` already — the asymmetry makes the pattern harder to audit. Consider escaping all interpolated values uniformly for consistency.

- [Low] **Redundant `int()` / `float()` / `str()` casts in `get_summary` (metrics.py:247-254).**
The values stored in `_clients` are already typed correctly at insertion time (lines 106-110). The explicit `str(data["name"])`, `float(data["last_seen"])`, `int(data["initialize_count"])` casts in the summary builder are defensive but add noise. Same applies to the `int(existing["initialize_count"]) + 1` on line 113 — the value is always an `int`. Not wrong, but could be simplified.

- [Low] **`client_identities` table has no index on `last_seen` (shared_metrics.py:84-92).**
The `ORDER BY last_seen DESC` query in `get_summary` (line 306) performs a full table scan. At expected cardinalities (<10 rows) this is negligible, but adding a covering index would be consistent with the indexing style used for `requests` and `param_patterns`.

### Architectural Notes

- The summary contract remains backward compatible: `client_name`/`client_version` are preserved alongside the new `clients` array. This is a clean additive evolution.
- The JS fallback path (lines 199-204) correctly synthesizes a single-element `clients` array from legacy `client_name`/`client_version` when the `clients` array is absent or empty. This ensures backward compatibility with older server versions.
- The `client_identities` SQLite table uses `ON CONFLICT ... DO UPDATE SET initialize_count = client_identities.initialize_count + 1`, which is atomic and correct for concurrent multi-process writes.
- CSS uses `auto-fit` grid with a reasonable `minmax(220px, 1fr)`, which scales well for 1-4 clients.

### Tests

- New tests cover: multi-client summary in `MetricsCollector`, `SharedMetricsStore`, and the `/api/metrics` endpoint.
- Tests verify: empty clients list, single client, multiple clients, increment of `initialize_count`, and reset behavior.
- Full quality gates passed during EXECUTE: 585 tests, 92.18% coverage, ruff + mypy clean.
- No test coverage gaps observed for the new code paths.

### Next Steps

- [Actionable] Cap `_clients` dict and prune `client_identities` table to prevent unbounded growth (High).
- [Optional] Add `last_seen` index to `client_identities` table (Low).
- [Optional] Uniform escaping in `renderClientWidgets` for consistency (Medium).
4 changes: 2 additions & 2 deletions SPECS/INPROGRESS/next.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@

## Recently Archived

- 2026-02-18 — FU-P12-T1-3: Show multi-client widgets in Web UI instead of single overwritten active client (PASS)
- 2026-02-18 — FU-P12-T1-2: Add code comment clarifying stdin-only client capture in `on_request` (PASS)
- 2026-02-18 — FU-P12-T1-1: Remove or document `MCPInitializeParams` in schemas (PASS)
- 2026-02-18 — FU-BUG-T7-1: Cap `pending_methods` map to guard against unbounded growth (PASS)
- 2026-02-18 — FU-P13-T2-2: Move PID file write to after successful upstream launch (PASS)
- 2026-02-18 — FU-P13-T2-1: Replace run_forever() polling loop with asyncio.Event-based wait (PASS)
- 2026-02-18 — FU-P13-T4-2: Implement or remove reconnect parameter in BrokerProxy (PASS)

## Suggested Next Tasks

- P13-T5 follow-up — Complete interactive prompt verification in a desktop session (P1)
- FU-P12-T1-3Show multi-client widgets in Web UI instead of single overwritten active client (P2)
- FU-P12-T1-4Make `IN FLIGHT` KPI reflect real in-flight requests in shared-metrics mode (P2)
- FU-P12-T3-2 — Add `error_code` column to audit CSV export (P3)
40 changes: 36 additions & 4 deletions SPECS/Workplan.md
Original file line number Diff line number Diff line change
Expand Up @@ -2231,7 +2231,8 @@ Phase 9 Follow-up Backlog

---

#### FU-P12-T1-3: Show multi-client widgets in Web UI instead of single overwritten active client
#### ✅ FU-P12-T1-3: Show multi-client widgets in Web UI instead of single overwritten active client
- **Status:** ✅ Completed (2026-02-18)
- **Description:** The dashboard currently displays one `ACTIVE CLIENT` value that is overwritten by the most recent `initialize` handshake. Add multi-client visibility so the UI can show one widget/card per detected client (e.g., Codex, Zed, Cursor) with useful metadata (last seen and/or call counts), rather than a single global value.
- **Priority:** P2
- **Dependencies:** P12-T1
Expand All @@ -2242,9 +2243,40 @@ Phase 9 Follow-up Backlog
- Updated `src/mcpbridge_wrapper/webui/static/index.html` and `src/mcpbridge_wrapper/webui/static/dashboard.js` to render one widget per client
- Updated Web UI tests covering multi-client display behavior
- **Acceptance Criteria:**
- [ ] Dashboard shows multiple clients simultaneously when more than one client connects
- [ ] Existing single-client behavior remains correct when only one client is present
- [ ] Client widgets update in real time with the same refresh cadence as other KPIs
- [x] Dashboard shows multiple clients simultaneously when more than one client connects
- [x] Existing single-client behavior remains correct when only one client is present
- [x] Client widgets update in real time with the same refresh cadence as other KPIs
- [x] `pytest` suite remains green

---

#### FU-P12-T1-5: Cap `_clients` dict and prune `client_identities` to prevent unbounded growth
- **Description:** The in-memory `_clients` dict in `MetricsCollector` and the `client_identities` SQLite table in `SharedMetricsStore` grow without limit — every unique `(name, version)` pair adds an entry that is never evicted. Add a soft cap (e.g. 50 entries, evict oldest by `last_seen`) to `_clients`, and add a `WHERE last_seen > ?` pruning clause for `client_identities` on write. This aligns with the project pattern established by FU-BUG-T7-1 (`pending_methods` cap).
- **Priority:** P2
- **Dependencies:** FU-P12-T1-3
- **Parallelizable:** yes
- **Outputs/Artifacts:**
- Updated `src/mcpbridge_wrapper/webui/metrics.py` — soft cap on `_clients` dict
- Updated `src/mcpbridge_wrapper/webui/shared_metrics.py` — pruning old `client_identities` rows
- Updated tests covering eviction behavior
- **Acceptance Criteria:**
- [ ] `_clients` dict never exceeds the configured cap
- [ ] Stale `client_identities` rows are pruned on write
- [ ] Existing multi-client dashboard behavior is preserved
- [ ] `pytest` suite remains green

---

#### FU-P12-T1-6: Uniform HTML escaping in `renderClientWidgets`
- **Description:** In `dashboard.js` `renderClientWidgets`, the `count` integer and `lastSeen` string are interpolated into innerHTML without `escapeHtml()`, while `name` and `version` are escaped. Although `count` is always a number and `lastSeen` already passes through `escapeHtml` inside `formatRelativeAge`, the asymmetric pattern makes security auditing harder. Apply `escapeHtml()` uniformly to all interpolated values for consistency.
- **Priority:** P3
- **Dependencies:** FU-P12-T1-3
- **Parallelizable:** yes
- **Outputs/Artifacts:**
- Updated `src/mcpbridge_wrapper/webui/static/dashboard.js` — uniform escaping in `renderClientWidgets`
- **Acceptance Criteria:**
- [ ] All interpolated values in `renderClientWidgets` are passed through `escapeHtml()`
- [ ] No visual regression in client widget rendering
- [ ] `pytest` suite remains green

---
Expand Down
28 changes: 28 additions & 0 deletions src/mcpbridge_wrapper/webui/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ def __init__(self, window_seconds: int = 3600, max_datapoints: int = 3600) -> No
# MCP client identification
self._client_name: str = "unknown"
self._client_version: str = "unknown"
self._clients: Dict[Tuple[str, str], Dict[str, Any]] = {}

# Error breakdown by code
self._error_counts_by_code: Dict[int, int] = {}
Expand All @@ -95,8 +96,21 @@ def set_client_info(self, name: str, version: str) -> None:
version: Client version string (e.g. "1.2.3").
"""
with self._lock:
now = time.time()
self._client_name = name
self._client_version = version
key = (name, version)
existing = self._clients.get(key)
if existing is None:
self._clients[key] = {
"name": name,
"version": version,
"last_seen": now,
"initialize_count": 1,
}
else:
existing["last_seen"] = now
existing["initialize_count"] = existing["initialize_count"] + 1

def record_request(self, tool_name: str, request_id: Optional[str] = None) -> None:
"""Record an incoming request for a tool.
Expand Down Expand Up @@ -228,6 +242,18 @@ def get_summary(self) -> Dict[str, Any]:
"count": n,
}

clients: List[Dict[str, Any]] = []
for data in self._clients.values():
clients.append(
{
"name": data["name"],
"version": data["version"],
"last_seen": data["last_seen"],
"initialize_count": data["initialize_count"],
}
)
clients.sort(key=lambda item: item["last_seen"], reverse=True)

return {
"uptime_seconds": round(uptime, 1),
"total_requests": self._total_requests,
Expand All @@ -240,6 +266,7 @@ def get_summary(self) -> Dict[str, Any]:
"in_flight": len(self._in_flight),
"client_name": self._client_name,
"client_version": self._client_version,
"clients": clients,
"error_counts_by_code": dict(self._error_counts_by_code),
}

Expand Down Expand Up @@ -313,5 +340,6 @@ def reset(self) -> None:
self._in_flight.clear()
self._client_name = "unknown"
self._client_version = "unknown"
self._clients.clear()
self._error_counts_by_code.clear()
self._param_patterns.clear()
Loading