diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c3674fc..def68fc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,7 +10,7 @@ jobs: lint: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.2.2 with: persist-credentials: false @@ -34,7 +34,7 @@ jobs: CLICK_TO_MCP_NO_LICENSE: "1" steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.2.2 with: persist-credentials: false diff --git a/.github/workflows/cowork-auto-pr.yml b/.github/workflows/cowork-auto-pr.yml index b27f04e..a699aaf 100644 --- a/.github/workflows/cowork-auto-pr.yml +++ b/.github/workflows/cowork-auto-pr.yml @@ -16,7 +16,7 @@ jobs: # without this step every run failed with "not a git repository" and no # PR was ever opened (fleet-wide defect: 11/11 seeded copies lacked it). - name: Check out the pushed branch - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.2.2 with: ref: ${{ github.ref_name }} fetch-depth: 0 diff --git a/.github/workflows/pages.yml b/.github/workflows/pages.yml index 13639ed..1070199 100644 --- a/.github/workflows/pages.yml +++ b/.github/workflows/pages.yml @@ -18,7 +18,7 @@ jobs: build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.2.2 with: persist-credentials: false - name: Setup Pages diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index e098b68..42c1ddc 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -13,7 +13,7 @@ jobs: id-token: write steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.2.2 with: persist-credentials: false diff --git a/click_to_mcp/discover.py b/click_to_mcp/discover.py index 158d856..dec4906 100644 --- a/click_to_mcp/discover.py +++ b/click_to_mcp/discover.py @@ -8,10 +8,13 @@ from __future__ import annotations import importlib +import logging from dataclasses import dataclass from importlib.metadata import distribution, entry_points from typing import Any +logger = logging.getLogger(__name__) + @dataclass class DiscoveredCLI: @@ -47,7 +50,8 @@ def _get_package_metadata(pkg_name: str) -> str: try: dist = distribution(pkg_name) return dist.metadata.get("Summary", "") or "" - except Exception: + except Exception as exc: + logger.debug("Failed to get metadata for package %r: %s", pkg_name, exc) return "" @@ -88,7 +92,13 @@ def scan_entry_points() -> list[DiscoveredCLI]: is_typer=(cli_type == "typer"), ) ) - except (Exception, SystemExit): + except (Exception, SystemExit) as exc: + logger.debug( + "Skipping entry point %r (%s): %s", + entry_point.name, + getattr(entry_point, "module", "?"), + exc, + ) continue return discovered @@ -113,8 +123,21 @@ def load_cli(cli_name: str) -> Any | None: if entry_point.name == cli_name: try: return entry_point.load() - except Exception: + except Exception as exc: + logger.debug("Failed to load entry point %r: %s", cli_name, exc) return None + + # Fallback: built-in demo CLI bundled with this package. + # When running from a source checkout without `pip install -e .`, + # the entry point may not be registered, but we can still import directly. + if cli_name == "click-to-mcp-demo": + try: + from .demo import cli as demo_cli + return demo_cli + except Exception as exc: + logger.debug("Failed to import built-in demo CLI: %s", exc) + return None + return None @@ -138,7 +161,10 @@ def import_cli(module_path: str, attr_name: str) -> Any | None: if obj is not None: return obj return module - except Exception: + except Exception as exc: + logger.debug( + "Failed to import CLI from %s.%s: %s", module_path, attr_name, exc + ) return None @@ -168,7 +194,8 @@ def find_our_clis() -> dict[str, Any]: obj = ep.load() if _probe_cli_type(obj) != "unknown": result[ep.name] = obj - except Exception: + except Exception as exc: + logger.debug("Failed to load our CLI %r: %s", ep.name, exc) continue return result diff --git a/tests/test_discover_logging.py b/tests/test_discover_logging.py new file mode 100644 index 0000000..7ee4a06 --- /dev/null +++ b/tests/test_discover_logging.py @@ -0,0 +1,73 @@ +"""Tests for observability in discover.py — silent exception handlers should log.""" + +from __future__ import annotations + +import logging +from unittest.mock import MagicMock, patch + +import pytest + +from click_to_mcp.discover import ( + find_our_clis, + import_cli, + load_cli, + scan_entry_points, +) + + +class TestDiscoverLogging: + """Verify that silent exception paths emit diagnostic log messages.""" + + def test_scan_entry_points_logs_load_failure(self, caplog: pytest.LogCaptureFixture) -> None: + """scan_entry_points should log when an entry point fails to load.""" + fake_ep = MagicMock() + fake_ep.name = "broken-cli" + fake_ep.module = "nonexistent.module" + fake_ep.attr = "cli" + fake_ep.dist = None + fake_ep.load.side_effect = ImportError("no module named 'nonexistent'") + + with patch("click_to_mcp.discover.entry_points") as mock_eps: + mock_eps.return_value.select.return_value = [fake_ep] + with caplog.at_level(logging.DEBUG, logger="click_to_mcp.discover"): + result = scan_entry_points() + + assert result == [] + assert any("broken-cli" in r.message for r in caplog.records) + + def test_load_cli_logs_entry_point_failure(self, caplog: pytest.LogCaptureFixture) -> None: + """load_cli should log when an entry point load raises.""" + fake_ep = MagicMock() + fake_ep.name = "my-broken-tool" + fake_ep.load.side_effect = RuntimeError("entry point crashed") + + with patch("click_to_mcp.discover.entry_points") as mock_eps: + mock_eps.return_value.select.return_value = [fake_ep] + with caplog.at_level(logging.DEBUG, logger="click_to_mcp.discover"): + result = load_cli("my-broken-tool") + + assert result is None + assert any("my-broken-tool" in r.message for r in caplog.records) + + def test_import_cli_logs_import_failure(self, caplog: pytest.LogCaptureFixture) -> None: + """import_cli should log when module import fails.""" + with caplog.at_level(logging.DEBUG, logger="click_to_mcp.discover"): + result = import_cli("totally.fake.module.xyz", "app") + + assert result is None + assert any("totally.fake.module.xyz" in r.message for r in caplog.records) + + def test_find_our_clis_logs_load_failure(self, caplog: pytest.LogCaptureFixture) -> None: + """find_our_clis should log when a known-module entry point fails to load.""" + fake_ep = MagicMock() + fake_ep.name = "json2sql" + fake_ep.module = "json2sql.cli" + fake_ep.load.side_effect = AttributeError("missing attr") + + with patch("click_to_mcp.discover.entry_points") as mock_eps: + mock_eps.return_value.select.return_value = [fake_ep] + with caplog.at_level(logging.DEBUG, logger="click_to_mcp.discover"): + result = find_our_clis() + + assert result == {} + assert any("json2sql" in r.message for r in caplog.records)