Claude/refactor mcp bacnet xrpl o

[makeitworkok]

No description provided.

[Copilot]

Pull request overview

Refactors mcp4bas from a single-process “all-protocols” MCP server into an orchestrator intended to discover network context at startup, spawn sibling MCP servers, and proxy their tools through one MCP connection.

Changes:

• Replaces the legacy tool registry + in-process protocol connectors with an orchestrator-style mcp4bas.server (dynamic tool registration + get_network_context).
• Removes protocol connector implementations (BACnet/Modbus/MQTT/Haystack/SNMP), their resource datasets, and most associated tests.
• Updates packaging metadata (version bump, dependency changes, new console script + wheel package list entries).

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
tests/test_snmp_connector.py	Removed SNMP connector tests.
tests/test_server.py	Replaced broad tool/registry tests with minimal orchestrator + get_network_context assertions.
tests/test_mqtt_integration_fixtures.py	Removed MQTT fixture-based integration tests.
tests/test_mqtt_connector.py	Removed MQTT connector tests.
tests/test_modbus_connector.py	Removed Modbus connector tests.
tests/test_integration_adapters.py	Removed adapter-resolution integration test coverage for multiple protocols.
tests/test_haystack_connector.py	Removed Haystack connector/tag-validation tests.
tests/test_bacnet_connector.py	Removed BACnet connector + normalization tests.
src/mcp4bas/tools/core.py	Removed the Pydantic-based tool registry/handlers.
src/mcp4bas/tools/init.py	Replaced exports with a placeholder package docstring.
src/mcp4bas/snmp/connector.py	Removed SNMP connector implementation.
src/mcp4bas/snmp/init.py	Removed SNMP package exports.
src/mcp4bas/server.py	Implemented orchestrator lifecycle, dynamic proxy tool registration, and get_network_context.
src/mcp4bas/resources/snmp_dataset.json	Removed SNMP simulation dataset.
src/mcp4bas/resources/mqtt_messages.json	Removed MQTT seed dataset.
src/mcp4bas/resources/haystack_points.json	Removed Haystack dataset.
src/mcp4bas/resources/init.py	Removed SNMP dataset asset registration.
src/mcp4bas/mqtt/connector.py	Removed MQTT connector implementation.
src/mcp4bas/mqtt/init.py	Removed MQTT package exports.
src/mcp4bas/modbus/connector.py	Removed Modbus connector implementation.
src/mcp4bas/modbus/init.py	Removed Modbus package exports.
src/mcp4bas/haystack/connector.py	Removed Haystack connector implementation.
src/mcp4bas/haystack/init.py	Removed Haystack package exports.
src/mcp4bas/bacnet/connector.py	Removed BACnet connector implementation.
src/mcp4bas/bacnet/init.py	Removed BACnet package exports.
pyproject.toml	Bumped version, adjusted deps/extras, added mcp4bacnet script and wheel packages list entry.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

pyproject.toml declares a mcp4bacnet console script and includes src/mcp4bacnet in the wheel packages, but there is no src/mcp4bacnet/ directory in this branch. This will break packaging/install. Either add the new mcp4bacnet package, or remove/update the script + packages entries. Also, CI installs dev-requirements.txt (-e .[dev,dashboard]), but the dashboard extra was removed here; dependency installation will fail unless restored or the requirements file is updated.

[Copilot]

mcp4bas.server now imports OrchestratorConfig from mcp4bas.config, but there is no src/mcp4bas/config.py in this branch, so importing the server will raise ModuleNotFoundError and break runtime + tests. Add the missing module (and its public API), or update the imports to the correct existing location.

Suggested change

	from mcp4bas.config import OrchestratorConfig
	try:
	# Preferred: use the shared configuration model if available.
	from mcp4bas.config import OrchestratorConfig
	except ModuleNotFoundError: # pragma: no cover - compatibility for branches without mcp4bas.config
	import os
	from dataclasses import dataclass, field
	from pathlib import Path
	from typing import Mapping
	@dataclass
	class OrchestratorConfig:
	"""Fallback orchestrator configuration.
	This is used only when `mcp4bas.config` is not available in this branch.
	It derives its values purely from environment variables documented
	at the top of this module.
	"""
	siblings: Mapping[str, str] = field(default_factory=dict)
	verbose: bool = bool(int(os.environ.get("MCP4BAS_VERBOSE", "0") or "0"))
	network_cache: Path | None = (
	Path(os.environ["MCP4BAS_NETWORK_CACHE"])
	if os.environ.get("MCP4BAS_NETWORK_CACHE")
	else None
	)
	@classmethod
	def from_env(cls) -> "OrchestratorConfig":
	"""Construct configuration from MCP4BAS_* environment variables."""
	prefix = "MCP4BAS_SIBLING_"
	siblings: dict[str, str] = {}
	for key, value in os.environ.items():
	if not key.startswith(prefix):
	continue
	value = value.strip()
	if not value:
	continue
	# Normalise sibling name to lower-case for consistency.
	siblings[key[len(prefix) :].lower()] = value
	return cls(siblings=siblings)

[Copilot]

This file imports mcp4bas.network symbols (NetworkDiscovery, NetworkWatcher, discover_network, etc.), but there is no src/mcp4bas/network.py in the repository, so import mcp4bas.server will fail. Add the missing mcp4bas.network module (or adjust imports to an existing module).

Suggested change

	from mcp4bas.network import (
	NetworkDiscovery,
	NetworkWatcher,
	_VERBOSE as _NET_VERBOSE,
	discover_network,
	discover_network_context,
	startup_network_check,
	)
	try:
	from mcp4bas.network import (
	NetworkDiscovery,
	NetworkWatcher,
	_VERBOSE as _NET_VERBOSE,
	discover_network,
	discover_network_context,
	startup_network_check,
	)
	except ImportError:
	# Fallback stubs if mcp4bas.network is not available.
	# These preserve import-time behavior while making it clear at call-time
	# that the network functionality is not implemented in this environment.
	_NET_VERBOSE: bool = False
	class NetworkDiscovery:
	"""Placeholder for mcp4bas.network.NetworkDiscovery.
	The real implementation should be provided by the mcp4bas.network module.
	"""
	def __init__(self, *args: object, **kwargs: object) -> None: # type: ignore[no-untyped-def]
	raise RuntimeError(
	"NetworkDiscovery is unavailable because 'mcp4bas.network' "
	"could not be imported. Ensure the mcp4bas.network module is installed "
	"and importable."
	)
	class NetworkWatcher:
	"""Placeholder for mcp4bas.network.NetworkWatcher."""
	def __init__(self, *args: object, **kwargs: object) -> None: # type: ignore[no-untyped-def]
	raise RuntimeError(
	"NetworkWatcher is unavailable because 'mcp4bas.network' "
	"could not be imported. Ensure the mcp4bas.network module is installed "
	"and importable."
	)
	def discover_network(*args: object, **kwargs: object) -> object: # type: ignore[no-untyped-def]
	"""Placeholder for mcp4bas.network.discover_network."""
	raise RuntimeError(
	"discover_network is unavailable because 'mcp4bas.network' "
	"could not be imported. Ensure the mcp4bas.network module is installed "
	"and importable."
	)
	def discover_network_context(*args: object, **kwargs: object) -> object: # type: ignore[no-untyped-def]
	"""Placeholder for mcp4bas.network.discover_network_context."""
	raise RuntimeError(
	"discover_network_context is unavailable because 'mcp4bas.network' "
	"could not be imported. Ensure the mcp4bas.network module is installed "
	"and importable."
	)
	def startup_network_check(*args: object, **kwargs: object) -> object: # type: ignore[no-untyped-def]
	"""Placeholder for mcp4bas.network.startup_network_check."""
	raise RuntimeError(
	"startup_network_check is unavailable because 'mcp4bas.network' "
	"could not be imported. Ensure the mcp4bas.network module is installed "
	"and importable."
	)

[Copilot]

mcp4bas.server imports OrchestratorProxy from mcp4bas.proxy, but src/mcp4bas/proxy.py is missing in this branch. This will raise ModuleNotFoundError at import time; add the module or update the import to the actual implementation path.

Suggested change

	from mcp4bas.proxy import OrchestratorProxy
	try:
	# Prefer the real OrchestratorProxy implementation when available.
	from mcp4bas.proxy import OrchestratorProxy # type: ignore[attr-defined]
	except ModuleNotFoundError:
	class OrchestratorProxy:
	"""Fallback stub used when mcp4bas.proxy is unavailable.
	This avoids ModuleNotFoundError at import time but will fail fast when
	an orchestrator proxy is actually instantiated.
	"""
	def __init__(self, *args: Any, **kwargs: Any) -> None:
	raise RuntimeError(
	"mcp4bas.proxy.OrchestratorProxy is not available in this build. "
	"Ensure the 'mcp4bas.proxy' module is present or install a version "
	"of mcp4bas that includes it."
	)

[Copilot]

asyncio is imported but never used. CI runs ruff check src tests, so this will fail with F401 (unused import). Remove the import (or use it if intended).

Suggested change

import asyncio

[Copilot]

If an exception occurs after starting the proxy (and/or watcher) but before the yield (e.g., during tool registration), the shutdown section will not run because startup isn’t wrapped in try/finally. Wrap the startup portion of _lifespan in try/finally and ensure watcher.stop() / proxy.stop() are called on startup failures to avoid orphaned subprocesses/background tasks.

Suggested change

	proxy = OrchestratorProxy(config, discovery)
	discovered_tools = await proxy.start()
	_proxy = proxy
	# Step 3 -- subnet change watcher (restarts BACnet sibling on network move)
	async def _on_network_change(new_discovery: NetworkDiscovery) -> None:
	_LOGGER.warning(
	"subnet_changed old=%s new=%s gateway=%s -- restarting BACnet sibling",
	discovery.subnet,
	new_discovery.subnet,
	new_discovery.gateway,
	)
	if _proxy is not None:
	ok = await _proxy.restart_sibling("bacnet", new_discovery)
	if ok:
	_LOGGER.info("bacnet_sibling_restarted subnet=%s", new_discovery.subnet)
	else:
	_LOGGER.error(
	"bacnet_sibling_restart_failed -- BACnet may be unreachable on new subnet"
	)
	watcher = NetworkWatcher(interval_sec=600, on_change=_on_network_change)
	await watcher.start()
	_watcher = watcher
	# Step 4 -- register proxy tools dynamically
	for tool in discovered_tools:
	tool_name = tool.name
	tool_description = tool.description or tool_name
	def _make_handler(name: str):
	async def _handler(**kwargs: Any) -> dict[str, Any]:
	if _proxy is None:
	return {"status": "error", "message": "Proxy not initialized"}
	return await _proxy.call_tool(name, kwargs)
	_handler.__name__ = name
	_handler.__doc__ = tool_description
	return _handler
	server.add_tool(
	_make_handler(tool_name),
	name=tool_name,
	description=tool_description,
	)
	_LOGGER.info("orchestrator_ready tools=%d", len(discovered_tools))
	yield # Server is live
	# Shutdown
	await watcher.stop()
	_watcher = None
	await proxy.stop()
	_proxy = None
	proxy: OrchestratorProxy | None = None
	watcher: NetworkWatcher | None = None
	try:
	proxy = OrchestratorProxy(config, discovery)
	discovered_tools = await proxy.start()
	_proxy = proxy
	# Step 3 -- subnet change watcher (restarts BACnet sibling on network move)
	async def _on_network_change(new_discovery: NetworkDiscovery) -> None:
	_LOGGER.warning(
	"subnet_changed old=%s new=%s gateway=%s -- restarting BACnet sibling",
	discovery.subnet,
	new_discovery.subnet,
	new_discovery.gateway,
	)
	if _proxy is not None:
	ok = await _proxy.restart_sibling("bacnet", new_discovery)
	if ok:
	_LOGGER.info("bacnet_sibling_restarted subnet=%s", new_discovery.subnet)
	else:
	_LOGGER.error(
	"bacnet_sibling_restart_failed -- BACnet may be unreachable on new subnet"
	)
	watcher = NetworkWatcher(interval_sec=600, on_change=_on_network_change)
	await watcher.start()
	_watcher = watcher
	# Step 4 -- register proxy tools dynamically
	for tool in discovered_tools:
	tool_name = tool.name
	tool_description = tool.description or tool_name
	def _make_handler(name: str):
	async def _handler(**kwargs: Any) -> dict[str, Any]:
	if _proxy is None:
	return {"status": "error", "message": "Proxy not initialized"}
	return await _proxy.call_tool(name, kwargs)
	_handler.__name__ = name
	_handler.__doc__ = tool_description
	return _handler
	server.add_tool(
	_make_handler(tool_name),
	name=tool_name,
	description=tool_description,
	)
	_LOGGER.info("orchestrator_ready tools=%d", len(discovered_tools))
	yield # Server is live
	finally:
	# Shutdown (also on startup failure)
	if watcher is not None:
	try:
	await watcher.stop()
	finally:
	if _watcher is watcher:
	_watcher = None
	if proxy is not None:
	try:
	await proxy.stop()
	finally:
	if _proxy is proxy:
	_proxy = None

[Copilot]

_on_network_change logs old=%s using the initial discovery.subnet captured at startup, but that value is never updated after a change. After the first subnet move this makes the log message misleading. Consider tracking/updating a last-known discovery value after handling each change.

[Copilot]

get_network_context() re-runs live network discovery on each call, and this test invokes it directly, making the unit test environment-dependent and potentially flaky/slow in CI. Consider monkeypatching/stubbing the discovery functions (or injecting a discovery provider) so the test is deterministic.