diff --git a/SPECS/ARCHIVE/FU-P13-T19_Add_integration_coverage_for_broker-hosted_Web_UI_observability/FU-P13-T19_Add_integration_coverage_for_broker-hosted_Web_UI_observability.md b/SPECS/ARCHIVE/FU-P13-T19_Add_integration_coverage_for_broker-hosted_Web_UI_observability/FU-P13-T19_Add_integration_coverage_for_broker-hosted_Web_UI_observability.md new file mode 100644 index 00000000..c7202003 --- /dev/null +++ b/SPECS/ARCHIVE/FU-P13-T19_Add_integration_coverage_for_broker-hosted_Web_UI_observability/FU-P13-T19_Add_integration_coverage_for_broker-hosted_Web_UI_observability.md @@ -0,0 +1,76 @@ +# FU-P13-T19 PRD — Add integration coverage for broker-hosted Web UI observability + +## Objective + +Add an integration test that exercises broker-mode telemetry end-to-end and validates that Web UI APIs expose aggregated metrics and audit visibility for multiple broker-connected clients. The test must simulate realistic concurrent/near-concurrent client traffic over the broker Unix socket, ensure request/response telemetry is persisted through broker transport instrumentation, and verify dashboard API consumers can read that telemetry through `/api/metrics` and `/api/audit` without race-prone assertions. + +## Scope and Deliverables + +- Add or update integration tests under `tests/integration/webui/`. +- Reuse real broker components (`BrokerDaemon`, `UnixSocketServer`) with a deterministic upstream test script. +- Reuse Web UI API surface (`create_app`, `TestClient`) backed by shared metrics and audit implementations used by broker-hosted mode. +- Validate success-path and error-path telemetry visibility in API responses. +- Produce execution evidence in `SPECS/INPROGRESS/FU-P13-T19_Validation_Report.md`. + +## Success Criteria and Acceptance Tests + +### Functional Criteria + +- Aggregated metrics from broker-routed multi-client tool calls are visible via `/api/metrics`. +- At least one broker-routed error response is reflected in metrics (`total_errors`, tool error counters/error code breakdown) and audit output. +- Audit endpoint returns entries produced by broker telemetry logging (including request IDs and error fields when applicable). + +### Determinism / Stability Criteria + +- Assertions rely on request completion boundaries (client receives response) rather than arbitrary sleep durations. +- Any waiting for session cleanup or async propagation uses bounded polling with short timeout and clear failure messages. +- Test passes reliably in local runs and CI. + +## Test-First Plan + +1. Define/extend integration fixtures for: +- temporary short socket path (macOS-safe), +- deterministic upstream script that can emit both success and JSON-RPC error responses, +- broker startup/shutdown lifecycle. + +2. Implement failing integration test first: +- send requests from multiple clients through broker socket, +- include one explicit failing tool call, +- query Web UI APIs and assert aggregated visibility. + +3. Only adjust support code if tests expose legitimate integration gaps. If no production changes are needed, keep execution test-only. + +## Execution Plan + +### Phase 1: Build deterministic integration harness + +- Inputs: existing broker multi-client helper patterns and Web UI e2e setup patterns. +- Outputs: broker+webui fixture(s) and request helper(s) in integration test module. +- Verification: fixture starts broker, serves API app, and tears down cleanly. + +### Phase 2: Implement aggregated observability assertions + +- Inputs: multi-client request streams with distinct request IDs and tool names. +- Outputs: assertions for `/api/metrics` and `/api/audit` showing combined visibility across clients. +- Verification: expected totals, tool keys, and non-empty audit entries with matching request IDs. + +### Phase 3: Implement error-path observability assertions + +- Inputs: one synthetic tool failure from upstream. +- Outputs: assertions for error counters/rates and audit error fields (`error`, `error_code`). +- Verification: API output includes at least one error-classified entry tied to the failing call. + +## Decision Notes and Constraints + +- Prefer `SharedMetricsStore` over in-memory `MetricsCollector` to mirror broker-hosted deployment behavior. +- Keep upstream behavior inside temporary script fixture to avoid monkeypatching transport internals. +- Keep assertions tolerant to naturally computed latency values (check presence/non-negative rather than exact values). + +## Notes (Post-Implementation) + +- If test naming or organization changes, ensure discoverability remains under `tests/integration/webui/`. +- If CI runtime increases materially, record rationale and tradeoffs in the validation report. + +--- +**Archived:** 2026-02-28 +**Verdict:** PASS diff --git a/SPECS/ARCHIVE/FU-P13-T19_Add_integration_coverage_for_broker-hosted_Web_UI_observability/FU-P13-T19_Validation_Report.md b/SPECS/ARCHIVE/FU-P13-T19_Add_integration_coverage_for_broker-hosted_Web_UI_observability/FU-P13-T19_Validation_Report.md new file mode 100644 index 00000000..69473e24 --- /dev/null +++ b/SPECS/ARCHIVE/FU-P13-T19_Add_integration_coverage_for_broker-hosted_Web_UI_observability/FU-P13-T19_Validation_Report.md @@ -0,0 +1,36 @@ +# Validation Report: FU-P13-T19 — Add integration coverage for broker-hosted Web UI observability + +**Date:** 2026-02-28 +**Task ID:** FU-P13-T19 +**Verdict:** PASS + +## Scope Implemented + +- Added broker-hosted Web UI observability integration coverage: + - `tests/integration/webui/test_broker_observability.py` +- New integration scenario uses real broker components (`BrokerDaemon`, `UnixSocketServer`) with: + - deterministic upstream success and error responses, + - multi-client broker socket traffic, + - Web UI API assertions against `/api/metrics` and `/api/audit`. +- Coverage includes success-path aggregation and error-path telemetry (metrics + audit). + +## Quality Gates + +- `pytest -q` → FAIL in this environment (`ModuleNotFoundError: mcpbridge_wrapper` due src-layout import path) +- `PYTHONPATH=src pytest tests/integration/webui/test_broker_observability.py -q` → PASS (`1 passed`) +- `PYTHONPATH=src pytest` → PASS (`693 passed, 5 skipped, 2 warnings`) +- `python -m ruff check src/` → PASS (`All checks passed!`) +- `mypy src/` → PASS (`Success: no issues found in 18 source files`) +- `PYTHONPATH=src pytest --cov=src/mcpbridge_wrapper --cov-report=term-missing` → PASS + - Total coverage: **91.72%** (required: >= 90%) + +## Acceptance Criteria Check + +- [x] Tests demonstrate aggregated metrics visibility for broker-connected clients. +- [x] Tests cover at least one error-path request and verify error reporting in metrics/audit output. +- [x] CI remains stable without flaky timing assumptions. + +## Notes + +- The integration assertions are response-boundary driven (wait for per-request responses) and avoid arbitrary sleep-based timing assumptions. +- Error telemetry validation covers both metrics error breakdown (`error_counts_by_code`) and audit entry error fields for broker-routed failures. diff --git a/SPECS/ARCHIVE/INDEX.md b/SPECS/ARCHIVE/INDEX.md index 5cf6c411..d7207aaa 100644 --- a/SPECS/ARCHIVE/INDEX.md +++ b/SPECS/ARCHIVE/INDEX.md @@ -1,11 +1,12 @@ # mcpbridge-wrapper Tasks Archive -**Last Updated:** 2026-02-28 (FU-P13-T18_Document_unified_single-config_setup_for_broker_+_Web_UI_multi-agent_workflows) +**Last Updated:** 2026-02-28 (FU-P13-T19_Add_integration_coverage_for_broker-hosted_Web_UI_observability) ## Archived Tasks | Task ID | Folder | Archived | Verdict | |---------|--------|----------|---------| +| FU-P13-T19 | [FU-P13-T19_Add_integration_coverage_for_broker-hosted_Web_UI_observability/](FU-P13-T19_Add_integration_coverage_for_broker-hosted_Web_UI_observability/) | 2026-02-28 | PASS | | FU-P13-T18 | [FU-P13-T18_Document_unified_single-config_setup_for_broker_+_Web_UI_multi-agent_workflows/](FU-P13-T18_Document_unified_single-config_setup_for_broker_+_Web_UI_multi-agent_workflows/) | 2026-02-28 | PASS | | FU-P13-T17 | [FU-P13-T17_Enable_broker-hosted_Web_UI_with_shared_multi-client_telemetry/](FU-P13-T17_Enable_broker-hosted_Web_UI_with_shared_multi-client_telemetry/) | 2026-02-28 | PASS | | FU-P13-T16 | [FU-P13-T16_Document_multi_agent_MCP_usage_and_single_Web_UI_host/](FU-P13-T16_Document_multi_agent_MCP_usage_and_single_Web_UI_host/) | 2026-02-28 | PASS | @@ -165,6 +166,7 @@ | File | Description | |------|-------------| +| [REVIEW_fu_p13_t19_broker_webui_observability.md](_Historical/REVIEW_fu_p13_t19_broker_webui_observability.md) | Review report for FU-P13-T19 | | [REVIEW_fu_p13_t18_unified_config_docs.md](_Historical/REVIEW_fu_p13_t18_unified_config_docs.md) | Review report for FU-P13-T18 | | [REVIEW_fu_p13_t17_broker_hosted_webui.md](_Historical/REVIEW_fu_p13_t17_broker_hosted_webui.md) | Review report for FU-P13-T17 | | [REVIEW_fu_p13_t16_multi_agent_mcp_docs.md](_Historical/REVIEW_fu_p13_t16_multi_agent_mcp_docs.md) | Review report for FU-P13-T16 | @@ -279,6 +281,8 @@ | Date | Task ID | Action | |------|---------|--------| +| 2026-02-28 | FU-P13-T19 | Archived REVIEW_fu_p13_t19_broker_webui_observability report | +| 2026-02-28 | FU-P13-T19 | Archived Add_integration_coverage_for_broker-hosted_Web_UI_observability (PASS) | | 2026-02-28 | FU-P13-T18 | Archived REVIEW_fu_p13_t18_unified_config_docs report | | 2026-02-28 | FU-P13-T18 | Archived Document_unified_single-config_setup_for_broker_+_Web_UI_multi-agent_workflows (PASS) | | 2026-02-28 | FU-P13-T17 | Archived REVIEW_fu_p13_t17_broker_hosted_webui report | diff --git a/SPECS/ARCHIVE/_Historical/REVIEW_fu_p13_t19_broker_webui_observability.md b/SPECS/ARCHIVE/_Historical/REVIEW_fu_p13_t19_broker_webui_observability.md new file mode 100644 index 00000000..1876be2b --- /dev/null +++ b/SPECS/ARCHIVE/_Historical/REVIEW_fu_p13_t19_broker_webui_observability.md @@ -0,0 +1,36 @@ +## REVIEW REPORT — FU-P13-T19 Broker Web UI Observability + +**Scope:** origin/main..HEAD +**Files:** 6 + +### Summary Verdict +- [x] Approve +- [ ] Approve with comments +- [ ] Request changes +- [ ] Block + +### Critical Issues + +- None. + +### Secondary Issues + +- None. + +### Architectural Notes + +- The new integration path uses production broker transport + shared Web UI telemetry components, which improves confidence in the broker-hosted observability contract without introducing test-only behavior. +- The test asserts response-boundary outcomes instead of timing sleeps, which reduces flake risk for CI. + +### Tests + +- `pytest -q` fails in this environment due src-layout import resolution (`ModuleNotFoundError: mcpbridge_wrapper`). +- `PYTHONPATH=src pytest tests/integration/webui/test_broker_observability.py -q` passed. +- `PYTHONPATH=src pytest` passed (`693 passed, 5 skipped`). +- `python -m ruff check src/` passed. +- `mypy src/` passed. +- `PYTHONPATH=src pytest --cov=src/mcpbridge_wrapper --cov-report=term-missing` passed with **91.72%** coverage. + +### Next Steps + +- FOLLOW-UP skipped: no actionable review findings. diff --git a/SPECS/INPROGRESS/next.md b/SPECS/INPROGRESS/next.md index 50f25ecc..a517e12f 100644 --- a/SPECS/INPROGRESS/next.md +++ b/SPECS/INPROGRESS/next.md @@ -2,10 +2,10 @@ ## Recently Archived +- **FU-P13-T19** — Add integration coverage for broker-hosted Web UI observability (2026-02-28, PASS) - **FU-P13-T18** — Document unified single-config setup for broker + Web UI multi-agent workflows (2026-02-28, PASS) - **FU-P13-T17** — Enable broker-hosted Web UI with shared multi-client telemetry (2026-02-28, PASS) -- **FU-P13-T16** — Document multi-agent MCP usage and single Web UI host (2026-02-28, PASS) ## Suggested Next Tasks -- **FU-P13-T19** — Add integration coverage for broker-hosted Web UI observability +- **None** — All tasks completed (145/145) diff --git a/SPECS/Workplan.md b/SPECS/Workplan.md index 6ed515fb..bba72eef 100644 --- a/SPECS/Workplan.md +++ b/SPECS/Workplan.md @@ -2642,18 +2642,19 @@ Phase 9 Follow-up Backlog --- -#### ⬜️ FU-P13-T19: Add integration coverage for broker-hosted Web UI observability -- **Description:** Add automated test coverage that validates broker-mode telemetry visibility in Web UI APIs under multi-client request load. +#### ✅ FU-P13-T19: Add integration coverage for broker-hosted Web UI observability — Completed (2026-02-28, PASS) +- **Status:** ✅ Completed (2026-02-28, PASS) +- **Description:** Added integration coverage validating broker-hosted Web UI observability by exercising multi-client broker traffic and asserting aggregated/error telemetry via `/api/metrics` and `/api/audit`. - **Priority:** P1 - **Dependencies:** FU-P13-T17 - **Parallelizable:** yes - **Outputs/Artifacts:** - - New or updated integration tests under `tests/integration/webui/` - - Validation notes for deterministic multi-client telemetry assertions + - Added `tests/integration/webui/test_broker_observability.py` + - Added `SPECS/ARCHIVE/FU-P13-T19_Add_integration_coverage_for_broker-hosted_Web_UI_observability/FU-P13-T19_Validation_Report.md` - **Acceptance Criteria:** - - [ ] Tests demonstrate aggregated metrics visibility for broker-connected clients - - [ ] Tests cover at least one error-path request and verify error reporting in metrics/audit output - - [ ] CI remains stable without flaky timing assumptions + - [x] Tests demonstrate aggregated metrics visibility for broker-connected clients + - [x] Tests cover at least one error-path request and verify error reporting in metrics/audit output + - [x] CI remains stable without flaky timing assumptions --- diff --git a/tests/integration/webui/test_broker_observability.py b/tests/integration/webui/test_broker_observability.py new file mode 100644 index 00000000..d85da176 --- /dev/null +++ b/tests/integration/webui/test_broker_observability.py @@ -0,0 +1,233 @@ +"""Integration coverage for broker-hosted Web UI observability.""" + +from __future__ import annotations + +import asyncio +import contextlib +import json +import shutil +import sys +import tempfile +from pathlib import Path +from typing import Any, AsyncIterator, Iterator + +import pytest +import pytest_asyncio + +# Skip all tests if webui dependencies are not installed +pytest.importorskip("fastapi") +pytest.importorskip("uvicorn") + +from fastapi.testclient import TestClient + +from mcpbridge_wrapper.broker.daemon import BrokerDaemon +from mcpbridge_wrapper.broker.transport import UnixSocketServer +from mcpbridge_wrapper.broker.types import BrokerConfig +from mcpbridge_wrapper.webui.audit import AuditLogger +from mcpbridge_wrapper.webui.config import WebUIConfig +from mcpbridge_wrapper.webui.server import create_app +from mcpbridge_wrapper.webui.shared_metrics import SharedMetricsStore + + +@pytest.fixture +def telemetry_upstream_script(tmp_path: Path) -> Path: + """Create an upstream script that returns both success and error responses.""" + script = tmp_path / "upstream_telemetry.py" + script.write_text( + "import json\n" + "import sys\n" + "for raw in sys.stdin:\n" + " line = raw.strip()\n" + " if not line:\n" + " continue\n" + " msg = json.loads(line)\n" + " request_id = msg.get('id')\n" + " if request_id is None:\n" + " continue\n" + " params = msg.get('params', {}) if isinstance(msg.get('params'), dict) else {}\n" + " tool_name = params.get('name')\n" + " if tool_name == 'FailTool':\n" + " response = {\n" + " 'jsonrpc': '2.0',\n" + " 'id': request_id,\n" + " 'error': {'code': -32603, 'message': 'Synthetic upstream failure'},\n" + " }\n" + " else:\n" + " response = {\n" + " 'jsonrpc': '2.0',\n" + " 'id': request_id,\n" + " 'result': {\n" + " 'content': [{'type': 'text', 'text': 'ok'}],\n" + " 'tool': tool_name,\n" + " },\n" + " }\n" + " print(json.dumps(response, separators=(',', ':')), flush=True)\n", + encoding="utf-8", + ) + return script + + +@pytest.fixture +def broker_observability_runtime( + telemetry_upstream_script: Path, +) -> Iterator[dict[str, Any]]: + """Prepare broker/webui runtime resources with short, deterministic paths.""" + short_dir = Path(tempfile.mkdtemp(dir="/tmp", prefix="mcpbw")) + try: + db_path = short_dir / "metrics.db" + audit_dir = short_dir / "audit" + yield { + "short_dir": short_dir, + "db_path": db_path, + "audit_dir": audit_dir, + "broker_config": BrokerConfig( + socket_path=short_dir / "broker.sock", + pid_file=short_dir / "broker.pid", + upstream_cmd=[sys.executable, "-u", str(telemetry_upstream_script)], + reconnect_backoff_cap=1, + queue_ttl=2, + graceful_shutdown_timeout=1, + ), + } + finally: + shutil.rmtree(short_dir, ignore_errors=True) + + +@pytest_asyncio.fixture +async def running_broker_with_webui( + broker_observability_runtime: dict[str, Any], +) -> AsyncIterator[dict[str, Any]]: + """Start broker transport with shared metrics/audit and a TestClient app.""" + metrics = SharedMetricsStore(db_path=broker_observability_runtime["db_path"]) + audit = AuditLogger(log_dir=str(broker_observability_runtime["audit_dir"])) + config = WebUIConfig() + config._data["audit"]["log_dir"] = str(broker_observability_runtime["audit_dir"]) + + app = create_app(config, metrics, audit) + client = TestClient(app) + + broker_config: BrokerConfig = broker_observability_runtime["broker_config"] + daemon = BrokerDaemon(broker_config) + transport = UnixSocketServer(broker_config, daemon, metrics=metrics, audit=audit) + daemon._transport = transport # noqa: SLF001 + await daemon.start() + + try: + yield { + "daemon": daemon, + "transport": transport, + "broker_config": broker_config, + "client": client, + "metrics": metrics, + "audit": audit, + } + finally: + with contextlib.suppress(Exception): + await daemon.stop() + client.close() + audit.close() + metrics.close() + + +async def _send_client_tool_call( + broker_config: BrokerConfig, + *, + request_id: str, + client_name: str, + tool_name: str, +) -> dict[str, Any]: + """Send initialize + tools/call from one broker client and return tool response.""" + reader, writer = await asyncio.open_unix_connection(str(broker_config.socket_path)) + try: + initialize = { + "jsonrpc": "2.0", + "id": f"init-{request_id}", + "method": "initialize", + "params": {"clientInfo": {"name": client_name, "version": "1.0.0"}}, + } + tool_call = { + "jsonrpc": "2.0", + "id": request_id, + "method": "tools/call", + "params": {"name": tool_name, "arguments": {"request_id": request_id}}, + } + writer.write((json.dumps(initialize, separators=(",", ":")) + "\n").encode()) + writer.write((json.dumps(tool_call, separators=(",", ":")) + "\n").encode()) + await writer.drain() + + tool_response: dict[str, Any] | None = None + for _ in range(2): + raw = await asyncio.wait_for(reader.readline(), timeout=2.0) + assert raw, "Expected broker response line" + decoded = json.loads(raw.decode()) + if decoded.get("id") == request_id: + tool_response = decoded + + assert tool_response is not None, f"Missing tool response for {request_id}" + return tool_response + finally: + writer.close() + with contextlib.suppress(Exception): + await writer.wait_closed() + + +@pytest.mark.asyncio +async def test_broker_telemetry_visible_in_webui_api_under_multi_client_load( + running_broker_with_webui: dict[str, Any], +) -> None: + """Broker-routed multi-client traffic is aggregated in metrics/audit API views.""" + broker_config: BrokerConfig = running_broker_with_webui["broker_config"] + client: TestClient = running_broker_with_webui["client"] + + success_calls = 8 + calls = [ + _send_client_tool_call( + broker_config, + request_id=f"ok-{index}", + client_name=f"Client-{index % 3}", + tool_name="BuildProject", + ) + for index in range(success_calls) + ] + calls.append( + _send_client_tool_call( + broker_config, + request_id="err-0", + client_name="Client-error", + tool_name="FailTool", + ) + ) + + responses = await asyncio.gather(*calls) + assert sum(1 for response in responses if "result" in response) == success_calls + error_responses = [response for response in responses if "error" in response] + assert len(error_responses) == 1 + assert error_responses[0]["error"]["code"] == -32603 + + metrics_response = client.get("/api/metrics") + assert metrics_response.status_code == 200 + metrics_data = metrics_response.json() + + assert metrics_data["total_requests"] == 9 + assert metrics_data["total_errors"] == 1 + assert metrics_data["tool_counts"]["BuildProject"] == success_calls + assert metrics_data["tool_counts"]["FailTool"] == 1 + assert metrics_data["tool_errors"]["FailTool"] == 1 + error_breakdown = metrics_data["error_counts_by_code"] + assert error_breakdown.get("-32603", 0) == 1 + + client_names = {entry["name"] for entry in metrics_data["clients"]} + assert "Client-0" in client_names + assert "Client-1" in client_names + + audit_response = client.get("/api/audit?limit=100") + assert audit_response.status_code == 200 + audit_data = audit_response.json() + assert audit_data["total"] >= 9 + entries = audit_data["entries"] + assert any(entry.get("tool") == "BuildProject" for entry in entries) + + failing_entries = [entry for entry in entries if entry.get("tool") == "FailTool"] + assert failing_entries, "Expected at least one failing audit entry" + assert any(entry.get("error_code") == -32603 for entry in failing_entries) + assert any("Synthetic upstream failure" in str(entry.get("error")) for entry in failing_entries)