From 93786eb7e4538bf8a2bcd8a3f557f8ebfc3313ac Mon Sep 17 00:00:00 2001 From: Kirk Brauer Date: Sat, 29 Aug 2026 18:26:34 -0400 Subject: [PATCH 01/11] feat(cli): add jmp describe for exporters, leases, and clients Add a kubectl-style 'jmp describe' command group (alias: desc) with exporter, lease, and client subcommands. Default output is aligned plain-text key/value sections; -o json|yaml routes through model_print. 'describe client' reports token expiry and validity without ever printing token values, using a dedicated ClientDescription model for structured output. Assisted-by: Claude:claude-fable-5 Signed-off-by: Kirk Brauer --- .../jumpstarter_cli_common/alias.py | 1 + .../jumpstarter_cli/describe.py | 281 ++++++++++++++ .../jumpstarter_cli/describe_test.py | 342 ++++++++++++++++++ .../jumpstarter-cli/jumpstarter_cli/jmp.py | 2 + 4 files changed, 626 insertions(+) create mode 100644 python/packages/jumpstarter-cli/jumpstarter_cli/describe.py create mode 100644 python/packages/jumpstarter-cli/jumpstarter_cli/describe_test.py diff --git a/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/alias.py b/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/alias.py index e28ae58dc..9293d98b0 100644 --- a/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/alias.py +++ b/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/alias.py @@ -19,6 +19,7 @@ class AliasedGroup(click.Group): "move": ["mv"], "config": ["conf"], "delete": ["del", "d"], + "describe": ["desc"], "shell": ["sh", "s"], "exporter": ["exporters", "e"], "exporters": ["exporter"], diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/describe.py b/python/packages/jumpstarter-cli/jumpstarter_cli/describe.py new file mode 100644 index 000000000..e7e5c7db5 --- /dev/null +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/describe.py @@ -0,0 +1,281 @@ +from datetime import datetime, timezone + +import click +from jumpstarter_cli_common.alias import AliasedGroup +from jumpstarter_cli_common.config import opt_config +from jumpstarter_cli_common.exceptions import handle_exceptions, handle_exceptions_with_reauthentication +from jumpstarter_cli_common.oidc import decode_jwt, format_duration, get_token_remaining_seconds +from jumpstarter_cli_common.opt import OutputMode, OutputType +from jumpstarter_cli_common.print import model_print +from pydantic import BaseModel, ConfigDict, Field + +from .login import relogin_client +from jumpstarter.config.client import ClientConfigV1Alpha1 +from jumpstarter.config.user import UserConfigV1Alpha1 + +opt_output = click.option( + "-o", + "--output", + type=click.Choice([OutputMode.JSON, OutputMode.YAML]), + default=None, + help="Output mode. Defaults to a human-readable description.", +) + + +def _format_value(value) -> str: + if value is None or value == "": + return "" + if isinstance(value, bool): + return "yes" if value else "no" + if isinstance(value, datetime): + return value.strftime("%Y-%m-%d %H:%M:%S %Z").strip() + return str(value) + + +def _print_fields(fields: list[tuple[str, object]], indent: int = 0) -> None: + width = max(len(label) for label, _ in fields) + 1 + prefix = " " * indent + for label, value in fields: + click.echo(f"{prefix}{label + ':':<{width}} {_format_value(value)}") + + +def _print_mapping(label: str, mapping: dict[str, str], indent: int = 0) -> None: + prefix = " " * indent + if not mapping: + click.echo(f"{prefix}{label}: ") + return + click.echo(f"{prefix}{label}:") + for key, value in sorted(mapping.items()): + click.echo(f"{prefix} {key}={value}") + + +def _condition_time(condition) -> datetime | None: + if condition.HasField("lastTransitionTime"): + time = condition.lastTransitionTime + return datetime.fromtimestamp(time.seconds + time.nanos / 1e9, tz=timezone.utc) + return None + + +def _print_conditions(conditions) -> None: + if not conditions: + click.echo("Conditions: ") + return + click.echo("Conditions:") + headers = ["Type", "Status", "Reason", "Message", "Last Transition Time"] + rows = [ + [ + _format_value(condition.type), + _format_value(condition.status), + _format_value(condition.reason), + _format_value(condition.message), + _format_value(_condition_time(condition)), + ] + for condition in conditions + ] + widths = [max([len(header)] + [len(row[i]) for row in rows]) for i, header in enumerate(headers)] + dashes = ["-" * len(header) for header in headers] + for cells in [headers, dashes, *rows]: + click.echo(" " + " ".join(cell.ljust(width) for cell, width in zip(cells, widths, strict=True)).rstrip()) + + +@click.group(cls=AliasedGroup) +def describe(): + """ + Show detailed information about a specific resource + """ + + +@describe.command(name="exporter") +@opt_config(exporter=False) +@click.argument("name") +@opt_output +@handle_exceptions_with_reauthentication(relogin_client) +def describe_exporter(config, name: str, output: OutputType): + """ + Show details of a specific exporter + """ + + exporter = config.get_exporter(name) + + if output: + model_print(exporter, output) + return + + _print_fields( + [ + ("Name", exporter.name), + ("Namespace", exporter.namespace), + ] + ) + _print_mapping("Labels", exporter.labels) + if exporter.deprecated_labels: + _print_mapping("Deprecated Labels", exporter.deprecated_labels) + _print_fields( + [ + ("Online", exporter.online), + ("Status", str(exporter.status) if exporter.status else "UNKNOWN"), + ("Enabled", exporter.enabled), + ] + ) + if exporter.lease: + lease = exporter.lease + click.echo("Lease:") + _print_fields( + [ + ("Name", lease.name), + ("Client", lease.client), + ("Status", lease.get_status()), + ("Begin Time", lease.effective_begin_time or lease.begin_time), + ("End Time", lease.effective_end_time), + ("Duration", lease.duration), + ], + indent=2, + ) + else: + click.echo("Lease: ") + + +@describe.command(name="lease") +@opt_config(exporter=False) +@click.argument("name") +@opt_output +@handle_exceptions_with_reauthentication(relogin_client) +def describe_lease(config, name: str, output: OutputType): + """ + Show details of a specific lease + """ + + lease = config.get_lease(name=name) + + if output: + model_print(lease, output) + return + + _print_fields( + [ + ("Name", lease.name), + ("Namespace", lease.namespace), + ("Selector", lease.selector), + ("Exporter", lease.exporter), + ("Client", lease.client), + ("Status", lease.get_status()), + ("Duration", lease.duration), + ("Effective Begin Time", lease.effective_begin_time), + ("Effective End Time", lease.effective_end_time), + ] + ) + _print_mapping("Tags", lease.tags) + _print_mapping("Context", lease.context) + _print_conditions(lease.conditions) + + +class ClientDescription(BaseModel): + model_config = ConfigDict(populate_by_name=True) + + alias: str + path: str | None + current: bool + name: str | None + namespace: str | None + endpoint: str | None + tls_ca_configured: bool = Field(alias="tlsCaConfigured") + tls_insecure: bool = Field(alias="tlsInsecure") + drivers_allow: list[str] = Field(alias="driversAllow") + drivers_unsafe: bool = Field(alias="driversUnsafe") + token_expiry: datetime | None = Field(alias="tokenExpiry") + token_status: str = Field(alias="tokenStatus") + refresh_token_stored: bool = Field(alias="refreshTokenStored") + + +def _token_details(token: str | None) -> tuple[datetime | None, str]: + if not token: + return None, "no token" + try: + payload = decode_jwt(token) + except Exception: + return None, "malformed" + exp = payload.get("exp") + remaining = get_token_remaining_seconds(token) + if exp is None or remaining is None: + return None, "no expiry claim" + expiry = datetime.fromtimestamp(exp, tz=timezone.utc) + if remaining < 0: + return expiry, f"expired ({format_duration(remaining)} ago)" + return expiry, f"valid ({format_duration(remaining)} remaining)" + + +@describe.command(name="client") +@click.argument("alias", required=False, default=None) +@opt_output +@handle_exceptions +def describe_client(alias: str | None, output: OutputType): + """ + Show details of a client config + """ + + if alias is not None: + config = ClientConfigV1Alpha1.load(alias) + else: + config = UserConfigV1Alpha1.load_or_create().config.current_client + if config is None: + raise click.ClickException("no client alias specified and no default client is set") + + current_alias = ClientConfigV1Alpha1.list().current_config + + token_expiry, token_status = _token_details(config.token) + + description = ClientDescription( + alias=config.alias, + path=str(config.path) if config.path else None, + current=current_alias is not None and config.alias == current_alias, + name=config.metadata.name, + namespace=config.metadata.namespace, + endpoint=config.endpoint, + tls_ca_configured=bool(config.tls.ca), + tls_insecure=config.tls.insecure, + drivers_allow=config.drivers.allow, + drivers_unsafe=config.drivers.unsafe, + token_expiry=token_expiry, + token_status=token_status, + refresh_token_stored=bool(config.refresh_token), + ) + + if output: + model_print(description, output) + return + + _print_fields( + [ + ("Alias", description.alias), + ("Path", description.path), + ("Current", description.current), + ("Name", description.name), + ("Namespace", description.namespace), + ("Endpoint", description.endpoint), + ] + ) + click.echo("TLS:") + _print_fields( + [ + ("CA", "configured" if description.tls_ca_configured else None), + ("Insecure", description.tls_insecure), + ], + indent=2, + ) + click.echo("Drivers:") + _print_fields( + [ + ("Allow", ", ".join(description.drivers_allow) if description.drivers_allow else None), + ("Unsafe", description.drivers_unsafe), + ], + indent=2, + ) + click.echo("Token:") + _print_fields( + [ + ("Expiry", description.token_expiry), + ("Status", description.token_status), + ], + indent=2, + ) + _print_fields([("Refresh Token Stored", description.refresh_token_stored)]) diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/describe_test.py b/python/packages/jumpstarter-cli/jumpstarter_cli/describe_test.py new file mode 100644 index 000000000..90507c249 --- /dev/null +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/describe_test.py @@ -0,0 +1,342 @@ +import base64 +import json +import time +from contextlib import ExitStack +from datetime import datetime, timedelta, timezone +from pathlib import Path +from unittest.mock import MagicMock, patch + +from click.testing import CliRunner +from jumpstarter_protocol import kubernetes_pb2 + +from jumpstarter_cli.describe import describe + +from jumpstarter.client.grpc import Exporter, Lease +from jumpstarter.common import ExporterStatus +from jumpstarter.common.exceptions import ConnectionError + +CLIENT_TOKEN_PLACEHOLDER = "not-a-real-token" + + +def _make_jwt(exp_offset_seconds=3600, include_exp=True): + header = base64.urlsafe_b64encode(json.dumps({"alg": "none", "typ": "JWT"}).encode()).rstrip(b"=").decode() + payload_data = { + "sub": "test-subject", + "iss": "https://localhost:8085", + "iat": int(time.time()), + } + if include_exp: + payload_data["exp"] = int(time.time()) + exp_offset_seconds + payload = base64.urlsafe_b64encode(json.dumps(payload_data).encode()).rstrip(b"=").decode() + return f"{header}.{payload}.fake-signature" + + +def _make_condition(type="Ready", status="True", reason="Ready", message="lease is ready"): + condition = kubernetes_pb2.Condition(type=type, status=status, reason=reason, message=message) + condition.lastTransitionTime.seconds = int(datetime(2023, 1, 1, 10, 0, 0, tzinfo=timezone.utc).timestamp()) + return condition + + +def _make_lease(name="lease-1", conditions=None, **kwargs): + return Lease( + namespace="default", + name=name, + selector="board=rpi4", + duration=timedelta(minutes=30), + client="my-client", + exporter="exporter-1", + conditions=conditions if conditions is not None else [_make_condition()], + effective_begin_time=datetime(2023, 1, 1, 10, 0, 0, tzinfo=timezone.utc), + tags={"build": "1234"}, + context={"purpose": "ci"}, + **kwargs, + ) + + +def _make_exporter(lease=None): + return Exporter( + namespace="default", + name="exporter-1", + labels={"board": "rpi4", "env": "test"}, + online=True, + status=ExporterStatus.AVAILABLE, + enabled=True, + lease=lease, + ) + + +def _patch_remote_config(config): + mock_cls = MagicMock() + mock_cls.load.return_value = config + return patch("jumpstarter_cli_common.config.ClientConfigV1Alpha1", mock_cls) + + +def _make_client_config(alias="test-client", token=None, refresh_token=None): + config = MagicMock() + config.alias = alias + config.path = Path(f"/home/user/.config/jumpstarter/clients/{alias}.yaml") + config.metadata.name = "my-client" + config.metadata.namespace = "default" + config.endpoint = "grpc.example.com:443" + config.token = token + config.refresh_token = refresh_token + config.tls.ca = "" + config.tls.insecure = False + config.drivers.allow = ["jumpstarter_driver_power"] + config.drivers.unsafe = False + return config + + +def _patch_client_configs(config, current_alias=None, default_client=None): + client_cls = MagicMock() + client_cls.load.return_value = config + client_cls.list.return_value = MagicMock(current_config=current_alias) + user_cls = MagicMock() + user_cls.load_or_create.return_value.config.current_client = default_client + stack = ExitStack() + stack.enter_context(patch("jumpstarter_cli.describe.ClientConfigV1Alpha1", client_cls)) + stack.enter_context(patch("jumpstarter_cli.describe.UserConfigV1Alpha1", user_cls)) + return stack + + +class TestDescribeExporter: + def setup_method(self): + self.runner = CliRunner() + + def test_pretty_output(self): + config = MagicMock() + config.get_exporter.return_value = _make_exporter() + with _patch_remote_config(config): + result = self.runner.invoke(describe, ["exporter", "exporter-1", "--client", "test"]) + assert result.exit_code == 0, result.output + assert "Name:" in result.output + assert "exporter-1" in result.output + assert "Namespace:" in result.output + assert "board=rpi4" in result.output + assert "env=test" in result.output + assert "Online:" in result.output + assert "AVAILABLE" in result.output + assert "Enabled:" in result.output + assert "Lease: " in result.output + config.get_exporter.assert_called_once_with("exporter-1") + + def test_pretty_output_with_lease(self): + config = MagicMock() + config.get_exporter.return_value = _make_exporter(lease=_make_lease()) + with _patch_remote_config(config): + result = self.runner.invoke(describe, ["exporter", "exporter-1", "--client", "test"]) + assert result.exit_code == 0, result.output + assert "Lease:" in result.output + assert "lease-1" in result.output + assert "my-client" in result.output + assert "In-Use" in result.output + assert "0:30:00" in result.output + + def test_pretty_output_deprecated_labels(self): + exporter = _make_exporter() + exporter.deprecated_labels = {"old-key": "Use new-key instead"} + config = MagicMock() + config.get_exporter.return_value = exporter + with _patch_remote_config(config): + result = self.runner.invoke(describe, ["exporter", "exporter-1", "--client", "test"]) + assert result.exit_code == 0, result.output + assert "Deprecated Labels:" in result.output + assert "old-key=Use new-key instead" in result.output + + def test_json_output(self): + config = MagicMock() + config.get_exporter.return_value = _make_exporter() + with _patch_remote_config(config): + result = self.runner.invoke(describe, ["exporter", "exporter-1", "--client", "test", "-o", "json"]) + assert result.exit_code == 0, result.output + data = json.loads(result.output) + assert data["name"] == "exporter-1" + assert data["namespace"] == "default" + assert data["labels"] == {"board": "rpi4", "env": "test"} + assert data["online"] is True + + def test_unknown_name(self): + config = MagicMock() + config.get_exporter.side_effect = ConnectionError("exporter 'missing' not found") + with _patch_remote_config(config): + result = self.runner.invoke(describe, ["exporter", "missing", "--client", "test"]) + assert result.exit_code != 0 + assert "not found" in result.output + + +class TestDescribeLease: + def setup_method(self): + self.runner = CliRunner() + + def test_pretty_output(self): + config = MagicMock() + config.get_lease.return_value = _make_lease() + with _patch_remote_config(config): + result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test"]) + assert result.exit_code == 0, result.output + assert "Name:" in result.output + assert "lease-1" in result.output + assert "Selector:" in result.output + assert "board=rpi4" in result.output + assert "exporter-1" in result.output + assert "my-client" in result.output + assert "In-Use" in result.output + assert "0:30:00" in result.output + assert "Effective Begin Time:" in result.output + assert "build=1234" in result.output + assert "purpose=ci" in result.output + assert "Conditions:" in result.output + assert "Ready" in result.output + assert "lease is ready" in result.output + config.get_lease.assert_called_once_with(name="lease-1") + + def test_pretty_output_no_conditions(self): + config = MagicMock() + config.get_lease.return_value = _make_lease(conditions=[]) + with _patch_remote_config(config): + result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test"]) + assert result.exit_code == 0, result.output + assert "Conditions: " in result.output + assert "Unknown" in result.output + + def test_json_output(self): + config = MagicMock() + config.get_lease.return_value = _make_lease() + with _patch_remote_config(config): + result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test", "-o", "json"]) + assert result.exit_code == 0, result.output + data = json.loads(result.output) + assert data["name"] == "lease-1" + assert data["selector"] == "board=rpi4" + assert data["client"] == "my-client" + assert data["exporter"] == "exporter-1" + assert data["conditions"][0]["type"] == "Ready" + + def test_unknown_name(self): + config = MagicMock() + config.get_lease.side_effect = ConnectionError("lease 'missing' not found") + with _patch_remote_config(config): + result = self.runner.invoke(describe, ["lease", "missing", "--client", "test"]) + assert result.exit_code != 0 + assert "not found" in result.output + + +class TestDescribeClient: + def setup_method(self): + self.runner = CliRunner() + + def test_pretty_output(self): + token = _make_jwt(exp_offset_seconds=7200) + config = _make_client_config(token=token, refresh_token="fake-refresh") + with _patch_client_configs(config, current_alias="test-client"): + result = self.runner.invoke(describe, ["client", "test-client"]) + assert result.exit_code == 0, result.output + assert "Alias:" in result.output + assert "test-client" in result.output + assert "Current:" in result.output + assert "my-client" in result.output + assert "grpc.example.com:443" in result.output + assert "jumpstarter_driver_power" in result.output + assert "valid" in result.output + assert "Refresh Token Stored:" in result.output + assert token not in result.output + assert "fake-refresh" not in result.output + + def test_pretty_output_not_current(self): + config = _make_client_config() + with _patch_client_configs(config, current_alias="other-client"): + result = self.runner.invoke(describe, ["client", "test-client"]) + assert result.exit_code == 0, result.output + assert "Current:" in result.output + assert "no" in result.output + + def test_default_client(self): + config = _make_client_config() + with _patch_client_configs(config, current_alias="test-client", default_client=config): + result = self.runner.invoke(describe, ["client"]) + assert result.exit_code == 0, result.output + assert "test-client" in result.output + assert "yes" in result.output + + def test_no_default_client(self): + config = _make_client_config() + with _patch_client_configs(config, default_client=None): + result = self.runner.invoke(describe, ["client"]) + assert result.exit_code != 0 + assert "no default client" in result.output + + def test_json_output_never_contains_token(self): + token = _make_jwt(exp_offset_seconds=7200) + config = _make_client_config(token=token, refresh_token="fake-refresh") + with _patch_client_configs(config, current_alias="test-client"): + result = self.runner.invoke(describe, ["client", "test-client", "-o", "json"]) + assert result.exit_code == 0, result.output + data = json.loads(result.output) + assert data["alias"] == "test-client" + assert data["current"] is True + assert data["endpoint"] == "grpc.example.com:443" + assert data["tokenExpiry"] is not None + assert data["tokenStatus"].startswith("valid") + assert data["refreshTokenStored"] is True + assert token not in result.output + assert "fake-refresh" not in result.output + + def test_yaml_output_never_contains_token(self): + token = _make_jwt(exp_offset_seconds=7200) + config = _make_client_config(token=token, refresh_token="fake-refresh") + with _patch_client_configs(config, current_alias="test-client"): + result = self.runner.invoke(describe, ["client", "test-client", "-o", "yaml"]) + assert result.exit_code == 0, result.output + assert token not in result.output + assert "fake-refresh" not in result.output + assert "tokenStatus" in result.output + + def test_no_token(self): + config = _make_client_config(token=None) + with _patch_client_configs(config): + result = self.runner.invoke(describe, ["client", "test-client"]) + assert result.exit_code == 0, result.output + assert "no token" in result.output + + def test_malformed_token(self): + config = _make_client_config(token=CLIENT_TOKEN_PLACEHOLDER) + with _patch_client_configs(config): + result = self.runner.invoke(describe, ["client", "test-client"]) + assert result.exit_code == 0, result.output + assert "malformed" in result.output + assert "Traceback" not in result.output + + def test_malformed_token_json(self): + config = _make_client_config(token=CLIENT_TOKEN_PLACEHOLDER) + with _patch_client_configs(config): + result = self.runner.invoke(describe, ["client", "test-client", "-o", "json"]) + assert result.exit_code == 0, result.output + data = json.loads(result.output) + assert data["tokenStatus"] == "malformed" + assert data["tokenExpiry"] is None + + def test_token_without_exp_claim(self): + config = _make_client_config(token=_make_jwt(include_exp=False)) + with _patch_client_configs(config): + result = self.runner.invoke(describe, ["client", "test-client"]) + assert result.exit_code == 0, result.output + assert "no expiry claim" in result.output + + def test_expired_token(self): + config = _make_client_config(token=_make_jwt(exp_offset_seconds=-3600)) + with _patch_client_configs(config): + result = self.runner.invoke(describe, ["client", "test-client"]) + assert result.exit_code == 0, result.output + assert "expired" in result.output + + +class TestDescribeGroup: + def test_subcommands_registered(self): + assert set(describe.commands) == {"exporter", "lease", "client"} + + def test_desc_alias(self): + from jumpstarter_cli.jmp import jmp + + ctx = MagicMock() + ctx.fail = MagicMock() + assert jmp.get_command(ctx, "desc") is describe diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/jmp.py b/python/packages/jumpstarter-cli/jumpstarter_cli/jmp.py index 59c95fa03..befb3297c 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/jmp.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/jmp.py @@ -10,6 +10,7 @@ from .config import config from .create import create from .delete import delete +from .describe import describe from .get import get from .login import login from .run import run @@ -30,6 +31,7 @@ def jmp(): jmp.add_command(delete) jmp.add_command(update) jmp.add_command(get) +jmp.add_command(describe) jmp.add_command(shell) jmp.add_command(run) jmp.add_command(login) From 7a862edcec3026b5e0885e20e7a2e0077e613778 Mon Sep 17 00:00:00 2001 From: Kirk Brauer Date: Sat, 29 Aug 2026 18:27:14 -0400 Subject: [PATCH 02/11] refactor(client): move driver introspection into jumpstarter.client for CLI/MCP parity Move walk_click_tree, list_drivers, get_driver_methods and their helpers from jumpstarter_mcp.introspect into jumpstarter.client.introspect so the CLI (e.g. a future 'jmp describe lease --devices') can share them with the MCP server. Add describe_client plus describe_devices/describe_devices_async helpers that attach to an existing lease (never creating or releasing it) and return a plain-serializable devices dict. jumpstarter_mcp.introspect remains as a backward-compatible re-export shim. Assisted-by: Claude:claude-fable-5 Signed-off-by: Kirk Brauer --- .../jumpstarter_mcp/introspect.py | 230 ++----------- .../jumpstarter_mcp/server_test.py | 140 +------- .../jumpstarter_mcp/tools/commands.py | 3 +- .../jumpstarter/client/introspect.py | 278 +++++++++++++++ .../jumpstarter/client/introspect_test.py | 321 ++++++++++++++++++ 5 files changed, 635 insertions(+), 337 deletions(-) create mode 100644 python/packages/jumpstarter/jumpstarter/client/introspect.py create mode 100644 python/packages/jumpstarter/jumpstarter/client/introspect_test.py diff --git a/python/packages/jumpstarter-mcp/jumpstarter_mcp/introspect.py b/python/packages/jumpstarter-mcp/jumpstarter_mcp/introspect.py index b100040f0..7816a7551 100644 --- a/python/packages/jumpstarter-mcp/jumpstarter_mcp/introspect.py +++ b/python/packages/jumpstarter-mcp/jumpstarter_mcp/introspect.py @@ -1,205 +1,25 @@ -"""Introspection utilities for Click CLI trees and driver object trees.""" - -from __future__ import annotations - -import inspect -import logging -from typing import Any - -import click -import click.core - -logger = logging.getLogger(__name__) - - -def walk_click_tree(cmd: click.core.BaseCommand, path: list[str] | None = None) -> dict[str, Any]: # ty: ignore[unresolved-attribute] - """Recursively walk a Click command tree and return structured JSON. - - Returns command names, help text, parameters (with types and defaults), - and nested subcommands. - """ - path = path or [] - result: dict[str, Any] = { - "name": cmd.name, - "help": cmd.help, - "params": [ - { - "name": p.name, - "type": str(p.type), - "help": getattr(p, "help", None), - "required": getattr(p, "required", False), - "default": p.default if p.default is not None else None, - } - for p in cmd.params - if not getattr(p, "hidden", False) and p.name != "help" - ], - } - if isinstance(cmd, click.Group): - result["subcommands"] = { - name: walk_click_tree(sub, path + [name]) - for name, sub in cmd.commands.items() - } - return result - - -# Methods on the base DriverClient that should be excluded from introspection -_BASE_METHODS = { - "call", - "streamingcall", - "stream", - "stream_async", - "log_stream", - "log_stream_async", - "open_stream", - "close", - "reset", - "cli", - "call_async", - "streamingcall_async", - "report", - "report_async", - "get_status_async", - "end_session_async", - "status_monitor_async", -} - -# Methods inherited from base classes that add noise to driver listings -_INTERNAL_METHODS = { - "check_exporter_status", - "resource_async", - "wait_for_hook_complete_monitored", - "wait_for_hook_status", - "wait_for_lease_ready", - "wait_for_lease_ready_monitored", -} - - -def _get_public_method_names(obj: Any) -> list[str]: - """Return public method names for a driver instance, excluding base internals. - - Uses getmembers_static to avoid triggering property descriptors, which can - make gRPC calls that fail when invoked from the event loop thread. - """ - names = [] - try: - members = inspect.getmembers_static(obj) - except Exception: - logger.debug("inspect.getmembers_static failed for %s", type(obj).__name__, exc_info=True) - return names - for name, value in members: - if name.startswith("_") or name in _BASE_METHODS or name in _INTERNAL_METHODS: - continue - if isinstance(value, property): - continue - if not callable(value): - continue - try: - inspect.signature(value) - except (ValueError, TypeError, AttributeError): - continue - names.append(name) - return names - - -def list_drivers(client: Any, _keys: list[str] | None = None) -> list[dict[str, Any]]: - """Flatten the driver client tree into a list with dot-separated paths. - - Returns path (Python access path like ``client.power``), driver_path - (children-key list like ``["power"]`` for use with get_driver_methods), - class name, description, and method names for each driver. - """ - keys = _keys or [] - cls = type(client) - results = [ - { - "path": f"client.{'.'.join(keys)}" if keys else "client", - "driver_path": keys, - "class": f"{cls.__module__}.{cls.__qualname__}", - "description": getattr(client, "description", None), - "methods": _get_public_method_names(client), - } - ] - children = getattr(client, "children", {}) - for name, child in children.items(): - results.extend(list_drivers(child, keys + [name])) - return results - - -def _inspect_method(name: str, method: Any, driver_path: list[str]) -> dict[str, Any] | None: - """Build a method descriptor dict, or return None if the method can't be inspected.""" - try: - sig = inspect.signature(method) - except (ValueError, TypeError, AttributeError): - return None - - is_streaming = False - try: - source = inspect.getsource(method) - is_streaming = "streamingcall" in source - except (OSError, TypeError): - pass - - params = [ - { - "name": pname, - "annotation": str(p.annotation) if p.annotation != inspect.Parameter.empty else None, - "default": str(p.default) if p.default != inspect.Parameter.empty else None, - } - for pname, p in sig.parameters.items() - if pname != "self" - ] - - attr_path = ".".join(driver_path) - call_args = ", ".join(f"{p['name']}=..." for p in params) - method_call = f"client.{attr_path}.{name}({call_args})" - - return { - "name": name, - "signature": str(sig), - "docstring": inspect.getdoc(method), - "parameters": params, - "return_type": str(sig.return_annotation) if sig.return_annotation != inspect.Signature.empty else None, - "is_streaming": is_streaming, - "call_example": ( - "from jumpstarter.utils.env import env\n\n" - "with env() as client:\n" - f" {method_call}" - ), - } - - -def get_driver_methods(client: Any, driver_path: list[str]) -> dict[str, Any]: - """Inspect a specific driver client at the given path in the children tree. - - Uses Python inspect to return detailed method information for all public - methods defined on the concrete driver class. - """ - target = client - for key in driver_path: - children = getattr(target, "children", {}) - if key not in children: - raise KeyError(f"Driver path component '{key}' not found. Available: {list(children.keys())}") - target = children[key] - - cls = type(target) - try: - members = inspect.getmembers_static(target) - except Exception: - logger.debug("inspect.getmembers_static failed for %s", cls.__name__, exc_info=True) - members = [] - - methods = [] - for name, value in members: - if name.startswith("_") or name in _BASE_METHODS or name in _INTERNAL_METHODS: - continue - if isinstance(value, property) or not callable(value): - continue - info = _inspect_method(name, value, driver_path) - if info is not None: - methods.append(info) - - return { - "class": f"{cls.__module__}.{cls.__qualname__}", - "driver_path": driver_path, - "methods": methods, - } +"""Backward-compatible re-exports of the driver introspection utilities. + +The implementation lives in jumpstarter.client.introspect so it can be shared +between the CLI and the MCP server. +""" + +from jumpstarter.client.introspect import ( + _BASE_METHODS, + _INTERNAL_METHODS, + _get_public_method_names, + _inspect_method, + get_driver_methods, + list_drivers, + walk_click_tree, +) + +__all__ = [ + "_BASE_METHODS", + "_INTERNAL_METHODS", + "_get_public_method_names", + "_inspect_method", + "get_driver_methods", + "list_drivers", + "walk_click_tree", +] diff --git a/python/packages/jumpstarter-mcp/jumpstarter_mcp/server_test.py b/python/packages/jumpstarter-mcp/jumpstarter_mcp/server_test.py index afb646232..cc2fce59e 100644 --- a/python/packages/jumpstarter-mcp/jumpstarter_mcp/server_test.py +++ b/python/packages/jumpstarter-mcp/jumpstarter_mcp/server_test.py @@ -10,16 +10,9 @@ from datetime import datetime from unittest.mock import AsyncMock, MagicMock, patch -import click import pytest from jumpstarter_mcp.connections import Connection, ConnectionManager -from jumpstarter_mcp.introspect import ( - _get_public_method_names, - get_driver_methods, - list_drivers, - walk_click_tree, -) from jumpstarter_mcp.server import ( TOKEN_REFRESH_THRESHOLD_SECONDS, _ensure_fresh_token, @@ -112,135 +105,20 @@ class FakeLease: # --------------------------------------------------------------------------- -# walk_click_tree +# Introspection re-exports # --------------------------------------------------------------------------- -class TestWalkClickTree: - def test_simple_command(self): - @click.command("hello") - @click.option("--name", help="Your name") - def hello(name): - """Say hello.""" - - result = walk_click_tree(hello) - assert result["name"] == "hello" - assert result["help"] == "Say hello." - assert len(result["params"]) == 1 - assert result["params"][0]["name"] == "name" - assert result["params"][0]["help"] == "Your name" - assert "subcommands" not in result - - def test_group_with_subcommands(self): - @click.group("root") - def root(): - """Root group.""" - - @root.command("sub1") - def sub1(): - """First sub.""" - - @root.command("sub2") - @click.option("--count", type=int, default=5) - def sub2(count): - """Second sub.""" - - result = walk_click_tree(root) - assert result["name"] == "root" - assert "subcommands" in result - assert "sub1" in result["subcommands"] - assert "sub2" in result["subcommands"] - assert result["subcommands"]["sub2"]["params"][0]["name"] == "count" - assert result["subcommands"]["sub2"]["params"][0]["default"] == 5 - - def test_hidden_params_excluded(self): - @click.command("cmd") - @click.option("--visible", help="shown") - @click.option("--secret", hidden=True) - def cmd(visible, secret): - pass - - result = walk_click_tree(cmd) - names = [p["name"] for p in result["params"]] - assert "visible" in names - assert "secret" not in names - - -# Introspection tests -# --------------------------------------------------------------------------- +class TestIntrospectReexports: + def test_reexports_core_implementation(self): + import jumpstarter_mcp.introspect as shim + import jumpstarter.client.introspect as core -class TestGetPublicMethodNames: - def test_filters_private_and_base_methods(self): - names = _get_public_method_names(FakePowerClient()) - assert "on" in names - assert "off" in names - assert "cycle" in names - assert "_private" not in names - assert "call" not in names - assert "check_exporter_status" not in names - - -class TestListDrivers: - def test_flat_tree(self): - client = FakeCompositeClient() - result = list_drivers(client) - paths = [d["path"] for d in result] - assert "client" in paths - assert "client.power" in paths - assert "client.serial" in paths - - def test_driver_path_field(self): - client = FakeCompositeClient() - result = list_drivers(client) - root = next(d for d in result if d["path"] == "client") - assert root["driver_path"] == [] - power = next(d for d in result if d["path"] == "client.power") - assert power["driver_path"] == ["power"] - - def test_methods_populated(self): - client = FakeCompositeClient() - result = list_drivers(client) - power = next(d for d in result if d["path"] == "client.power") - assert "on" in power["methods"] - assert "off" in power["methods"] - - -class TestGetDriverMethods: - def test_returns_method_details(self): - client = FakeCompositeClient() - result = get_driver_methods(client, ["power"]) - assert result["driver_path"] == ["power"] - method_names = [m["name"] for m in result["methods"]] - assert "on" in method_names - assert "off" in method_names - assert "cycle" in method_names - - def test_call_example_uses_dot_notation(self): - client = FakeCompositeClient() - result = get_driver_methods(client, ["power"]) - cycle = next(m for m in result["methods"] if m["name"] == "cycle") - assert "client.power.cycle(" in cycle["call_example"] - assert 'children["power"]' not in cycle["call_example"] - - def test_invalid_path_raises(self): - client = FakeCompositeClient() - with pytest.raises(KeyError, match="nonexistent"): - get_driver_methods(client, ["nonexistent"]) - - def test_docstrings_captured(self): - client = FakeCompositeClient() - result = get_driver_methods(client, ["power"]) - on_method = next(m for m in result["methods"] if m["name"] == "on") - assert on_method["docstring"] == "Power on the device." - - def test_parameters_captured(self): - client = FakeCompositeClient() - result = get_driver_methods(client, ["power"]) - cycle = next(m for m in result["methods"] if m["name"] == "cycle") - assert len(cycle["parameters"]) == 1 - assert cycle["parameters"][0]["name"] == "wait" - assert cycle["parameters"][0]["default"] == "2" + assert shim.walk_click_tree is core.walk_click_tree + assert shim.list_drivers is core.list_drivers + assert shim.get_driver_methods is core.get_driver_methods + assert shim._get_public_method_names is core._get_public_method_names # --------------------------------------------------------------------------- diff --git a/python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/commands.py b/python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/commands.py index 8a8259da7..dc759f46d 100644 --- a/python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/commands.py +++ b/python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/commands.py @@ -12,7 +12,8 @@ import click from jumpstarter_mcp.connections import ConnectionManager -from jumpstarter_mcp.introspect import get_driver_methods, list_drivers, walk_click_tree + +from jumpstarter.client.introspect import get_driver_methods, list_drivers, walk_click_tree logger = logging.getLogger(__name__) diff --git a/python/packages/jumpstarter/jumpstarter/client/introspect.py b/python/packages/jumpstarter/jumpstarter/client/introspect.py new file mode 100644 index 000000000..6a4b17ea4 --- /dev/null +++ b/python/packages/jumpstarter/jumpstarter/client/introspect.py @@ -0,0 +1,278 @@ +"""Introspection utilities for Click CLI trees and driver object trees.""" + +from __future__ import annotations + +import inspect +import logging +from contextlib import ExitStack +from datetime import timedelta +from typing import TYPE_CHECKING, Any + +import click +import click.core +from anyio.from_thread import BlockingPortal, start_blocking_portal + +from .base import StubDriverClient +from .client import client_from_path + +if TYPE_CHECKING: + from jumpstarter.config.client import ClientConfigV1Alpha1 + +logger = logging.getLogger(__name__) + + +def _json_safe(value: Any) -> Any: + """Coerce a value to a JSON-serializable equivalent, stringifying as a last resort.""" + if value is None or isinstance(value, (bool, int, float, str)): + return value + if isinstance(value, (list, tuple)): + return [_json_safe(v) for v in value] + return str(value) + + +def walk_click_tree(cmd: click.core.BaseCommand, path: list[str] | None = None) -> dict[str, Any]: # ty: ignore[unresolved-attribute] + """Recursively walk a Click command tree and return structured JSON. + + Returns command names, help text, parameters (with types and defaults), + and nested subcommands. + """ + path = path or [] + result: dict[str, Any] = { + "name": cmd.name, + "help": cmd.help, + "params": [ + { + "name": p.name, + "type": str(p.type), + "help": getattr(p, "help", None), + "required": getattr(p, "required", False), + "default": _json_safe(p.default) if p.default is not None else None, + } + for p in cmd.params + if not getattr(p, "hidden", False) and p.name != "help" + ], + } + if isinstance(cmd, click.Group): + result["subcommands"] = { + name: walk_click_tree(sub, path + [name]) + for name, sub in cmd.commands.items() + } + return result + + +# Methods on the base DriverClient that should be excluded from introspection +_BASE_METHODS = { + "call", + "streamingcall", + "stream", + "stream_async", + "log_stream", + "log_stream_async", + "open_stream", + "close", + "reset", + "cli", + "call_async", + "streamingcall_async", + "report", + "report_async", + "get_status_async", + "end_session_async", + "status_monitor_async", +} + +# Methods inherited from base classes that add noise to driver listings +_INTERNAL_METHODS = { + "check_exporter_status", + "resource_async", + "wait_for_hook_complete_monitored", + "wait_for_hook_status", + "wait_for_lease_ready", + "wait_for_lease_ready_monitored", +} + + +def _get_public_method_names(obj: Any) -> list[str]: + """Return public method names for a driver instance, excluding base internals. + + Uses getmembers_static to avoid triggering property descriptors, which can + make gRPC calls that fail when invoked from the event loop thread. + """ + names = [] + try: + members = inspect.getmembers_static(obj) + except Exception: + logger.debug("inspect.getmembers_static failed for %s", type(obj).__name__, exc_info=True) + return names + for name, value in members: + if name.startswith("_") or name in _BASE_METHODS or name in _INTERNAL_METHODS: + continue + if isinstance(value, property): + continue + if not callable(value): + continue + try: + inspect.signature(value) + except (ValueError, TypeError, AttributeError): + continue + names.append(name) + return names + + +def list_drivers(client: Any, _keys: list[str] | None = None) -> list[dict[str, Any]]: + """Flatten the driver client tree into a list with dot-separated paths. + + Returns path (Python access path like ``client.power``), driver_path + (children-key list like ``["power"]`` for use with get_driver_methods), + class name, description, and method names for each driver. + """ + keys = _keys or [] + cls = type(client) + results = [ + { + "path": f"client.{'.'.join(keys)}" if keys else "client", + "driver_path": keys, + "class": f"{cls.__module__}.{cls.__qualname__}", + "description": getattr(client, "description", None), + "methods": _get_public_method_names(client), + } + ] + children = getattr(client, "children", {}) + for name, child in children.items(): + results.extend(list_drivers(child, keys + [name])) + return results + + +def _inspect_method(name: str, method: Any, driver_path: list[str]) -> dict[str, Any] | None: + """Build a method descriptor dict, or return None if the method can't be inspected.""" + try: + sig = inspect.signature(method) + except (ValueError, TypeError, AttributeError): + return None + + is_streaming = False + try: + source = inspect.getsource(method) + is_streaming = "streamingcall" in source + except (OSError, TypeError): + pass + + params = [ + { + "name": pname, + "annotation": str(p.annotation) if p.annotation != inspect.Parameter.empty else None, + "default": str(p.default) if p.default != inspect.Parameter.empty else None, + } + for pname, p in sig.parameters.items() + if pname != "self" + ] + + attr_path = ".".join(driver_path) + call_args = ", ".join(f"{p['name']}=..." for p in params) + method_call = f"client.{attr_path}.{name}({call_args})" + + return { + "name": name, + "signature": str(sig), + "docstring": inspect.getdoc(method), + "parameters": params, + "return_type": str(sig.return_annotation) if sig.return_annotation != inspect.Signature.empty else None, + "is_streaming": is_streaming, + "call_example": ( + "from jumpstarter.utils.env import env\n\n" + "with env() as client:\n" + f" {method_call}" + ), + } + + +def get_driver_methods(client: Any, driver_path: list[str]) -> dict[str, Any]: + """Inspect a specific driver client at the given path in the children tree. + + Uses Python inspect to return detailed method information for all public + methods defined on the concrete driver class. + """ + target = client + for key in driver_path: + children = getattr(target, "children", {}) + if key not in children: + raise KeyError(f"Driver path component '{key}' not found. Available: {list(children.keys())}") + target = children[key] + + cls = type(target) + try: + members = inspect.getmembers_static(target) + except Exception: + logger.debug("inspect.getmembers_static failed for %s", cls.__name__, exc_info=True) + members = [] + + methods = [] + for name, value in members: + if name.startswith("_") or name in _BASE_METHODS or name in _INTERNAL_METHODS: + continue + if isinstance(value, property) or not callable(value): + continue + info = _inspect_method(name, value, driver_path) + if info is not None: + methods.append(info) + + return { + "class": f"{cls.__module__}.{cls.__qualname__}", + "driver_path": driver_path, + "methods": methods, + } + + +def describe_client(client: Any) -> dict[str, Any]: + """Build a plain-serializable description of a connected driver client tree. + + Returns the flattened driver listing and the Click CLI tree. Drivers whose + client packages are not installed appear as StubDriverClient entries in the + listing, and cli_tree is None when the root client does not expose a CLI + (including when the root client itself is a stub). + """ + cli_tree = None + if not isinstance(client, StubDriverClient) and getattr(type(client), "cli", None) is not None: + cli_tree = walk_click_tree(client.cli()) + return { + "drivers": list_drivers(client), + "cli_tree": cli_tree, + } + + +async def describe_devices_async(config: ClientConfigV1Alpha1, lease_name: str) -> dict[str, Any]: + """Attach to an existing lease and describe its driver clients and CLI tree. + + Attaches to the lease named lease_name without creating a new lease and + without releasing it on exit, builds the root driver client, and returns + a plain-serializable dict with drivers and cli_tree keys. + + :raises ValueError: if lease_name is empty + :raises jumpstarter.client.exceptions.LeaseError: if the lease has ended + or cannot be acquired + :raises jumpstarter.common.exceptions.ConnectionError: if the lease does + not exist or the controller is unreachable + """ + if not lease_name: + raise ValueError("lease_name must be a non-empty existing lease name") + + async with BlockingPortal() as portal: + async with config.lease_async( + selector=None, + exporter_name=None, + lease_name=lease_name, + duration=timedelta(minutes=30), + portal=portal, + ) as lease: + async with lease.serve_unix_async() as path: + with ExitStack() as stack: + async with client_from_path( + path, portal, stack, allow=lease.allow, unsafe=lease.unsafe + ) as client: + return describe_client(client) + + +def describe_devices(config: ClientConfigV1Alpha1, lease_name: str) -> dict[str, Any]: + """Blocking convenience wrapper around describe_devices_async.""" + with start_blocking_portal() as portal: + return portal.call(describe_devices_async, config, lease_name) diff --git a/python/packages/jumpstarter/jumpstarter/client/introspect_test.py b/python/packages/jumpstarter/jumpstarter/client/introspect_test.py new file mode 100644 index 000000000..e5c5c4e32 --- /dev/null +++ b/python/packages/jumpstarter/jumpstarter/client/introspect_test.py @@ -0,0 +1,321 @@ +import json +from contextlib import asynccontextmanager +from unittest.mock import MagicMock, patch + +import click +import pytest + +from jumpstarter.client.introspect import ( + _get_public_method_names, + describe_client, + describe_devices, + describe_devices_async, + get_driver_methods, + list_drivers, + walk_click_tree, +) + + +class FakePowerClient: + children: dict = {} + + def on(self) -> None: + """Power on the device.""" + + def off(self) -> None: + """Power off the device.""" + + def cycle(self, wait: int = 2) -> None: + """Power cycle the device.""" + + def _private(self): + pass + + def call(self): + pass + + def check_exporter_status(self): + pass + + +class FakeSerialClient: + children: dict = {} + + def open(self): + """Open serial port.""" + + def pexpect(self): + """Create pexpect adapter.""" + + +class FakeCompositeClient: + description = "Test composite device" + + def __init__(self): + self.children = { + "power": FakePowerClient(), + "serial": FakeSerialClient(), + } + + def __getattr__(self, name): + try: + return self.children[name] + except KeyError: + raise AttributeError(name) from None + + def cli(self): + @click.group("j") + def base(): + """Fake composite device""" + + @base.command("on") + def on(): + """Power on.""" + + return base + + +class TestWalkClickTree: + def test_simple_command(self): + @click.command("hello") + @click.option("--name", help="Your name") + def hello(name): + """Say hello.""" + + result = walk_click_tree(hello) + assert result["name"] == "hello" + assert result["help"] == "Say hello." + assert len(result["params"]) == 1 + assert result["params"][0]["name"] == "name" + assert result["params"][0]["help"] == "Your name" + assert "subcommands" not in result + + def test_group_with_subcommands(self): + @click.group("root") + def root(): + """Root group.""" + + @root.command("sub1") + def sub1(): + """First sub.""" + + @root.command("sub2") + @click.option("--count", type=int, default=5) + def sub2(count): + """Second sub.""" + + result = walk_click_tree(root) + assert result["name"] == "root" + assert "subcommands" in result + assert "sub1" in result["subcommands"] + assert "sub2" in result["subcommands"] + assert result["subcommands"]["sub2"]["params"][0]["name"] == "count" + assert result["subcommands"]["sub2"]["params"][0]["default"] == 5 + + def test_hidden_params_excluded(self): + @click.command("cmd") + @click.option("--visible", help="shown") + @click.option("--secret", hidden=True) + def cmd(visible, secret): + pass + + result = walk_click_tree(cmd) + names = [p["name"] for p in result["params"]] + assert "visible" in names + assert "secret" not in names + + +class TestGetPublicMethodNames: + def test_filters_private_and_base_methods(self): + names = _get_public_method_names(FakePowerClient()) + assert "on" in names + assert "off" in names + assert "cycle" in names + assert "_private" not in names + assert "call" not in names + assert "check_exporter_status" not in names + + +class TestListDrivers: + def test_flat_tree(self): + client = FakeCompositeClient() + result = list_drivers(client) + paths = [d["path"] for d in result] + assert "client" in paths + assert "client.power" in paths + assert "client.serial" in paths + + def test_driver_path_field(self): + client = FakeCompositeClient() + result = list_drivers(client) + root = next(d for d in result if d["path"] == "client") + assert root["driver_path"] == [] + power = next(d for d in result if d["path"] == "client.power") + assert power["driver_path"] == ["power"] + + def test_methods_populated(self): + client = FakeCompositeClient() + result = list_drivers(client) + power = next(d for d in result if d["path"] == "client.power") + assert "on" in power["methods"] + assert "off" in power["methods"] + + +class TestGetDriverMethods: + def test_returns_method_details(self): + client = FakeCompositeClient() + result = get_driver_methods(client, ["power"]) + assert result["driver_path"] == ["power"] + method_names = [m["name"] for m in result["methods"]] + assert "on" in method_names + assert "off" in method_names + assert "cycle" in method_names + + def test_call_example_uses_dot_notation(self): + client = FakeCompositeClient() + result = get_driver_methods(client, ["power"]) + cycle = next(m for m in result["methods"] if m["name"] == "cycle") + assert "client.power.cycle(" in cycle["call_example"] + assert 'children["power"]' not in cycle["call_example"] + + def test_invalid_path_raises(self): + client = FakeCompositeClient() + with pytest.raises(KeyError, match="nonexistent"): + get_driver_methods(client, ["nonexistent"]) + + def test_docstrings_captured(self): + client = FakeCompositeClient() + result = get_driver_methods(client, ["power"]) + on_method = next(m for m in result["methods"] if m["name"] == "on") + assert on_method["docstring"] == "Power on the device." + + def test_parameters_captured(self): + client = FakeCompositeClient() + result = get_driver_methods(client, ["power"]) + cycle = next(m for m in result["methods"] if m["name"] == "cycle") + assert len(cycle["parameters"]) == 1 + assert cycle["parameters"][0]["name"] == "wait" + assert cycle["parameters"][0]["default"] == "2" + + +class TestDescribeClient: + def test_returns_drivers_and_cli_tree(self): + result = describe_client(FakeCompositeClient()) + paths = [d["path"] for d in result["drivers"]] + assert "client" in paths + assert "client.power" in paths + assert result["cli_tree"]["name"] == "j" + assert "on" in result["cli_tree"]["subcommands"] + + def test_cli_tree_none_without_cli(self): + result = describe_client(FakePowerClient()) + assert result["cli_tree"] is None + assert result["drivers"][0]["path"] == "client" + + def test_composite_client_e2e(self): + from jumpstarter_driver_composite.driver import Composite + from jumpstarter_driver_power.driver import MockPower + + from jumpstarter.common.utils import serve + + with serve( + Composite( + children={ + "power": MockPower(), + "nested": Composite( + children={ + "power": MockPower(), + }, + ), + }, + ) + ) as client: + result = describe_client(client) + + paths = [d["path"] for d in result["drivers"]] + assert "client" in paths + assert "client.power" in paths + assert "client.nested.power" in paths + power = next(d for d in result["drivers"] if d["path"] == "client.power") + assert power["driver_path"] == ["power"] + assert "on" in power["methods"] + assert "power" in result["cli_tree"]["subcommands"] + assert "nested" in result["cli_tree"]["subcommands"] + json.dumps(result) + + @pytest.mark.anyio + async def test_stub_client_represented(self): + from contextlib import ExitStack + + from anyio.from_thread import BlockingPortal + + from jumpstarter.client.base import StubDriverClient + + async with BlockingPortal() as portal: + stub = StubDriverClient( + labels={"jumpstarter.dev/client": "missing_pkg.client.MissingClient"}, + stub=None, + portal=portal, + stack=ExitStack(), + ) + result = describe_client(stub) + + assert result["cli_tree"] is None + assert result["drivers"][0]["class"].endswith("StubDriverClient") + + +class TestDescribeDevices: + @pytest.fixture() + def mock_config(self): + fake_lease = MagicMock() + fake_lease.allow = [] + fake_lease.unsafe = True + + @asynccontextmanager + async def fake_serve_unix_async(): + yield "/tmp/fake.sock" + + fake_lease.serve_unix_async = fake_serve_unix_async + + @asynccontextmanager + async def fake_lease_async(*args, **kwargs): + yield fake_lease + + config = MagicMock() + config.lease_async = MagicMock(side_effect=fake_lease_async) + return config + + @pytest.fixture() + def mock_client_from_path(self): + @asynccontextmanager + async def fake_client_from_path(path, portal, stack, allow, unsafe): + yield FakeCompositeClient() + + with patch("jumpstarter.client.introspect.client_from_path", fake_client_from_path): + yield + + @pytest.mark.anyio + async def test_attaches_to_named_lease(self, mock_config, mock_client_from_path): + result = await describe_devices_async(mock_config, "existing-lease") + + kwargs = mock_config.lease_async.call_args.kwargs + assert kwargs["lease_name"] == "existing-lease" + assert kwargs["selector"] is None + assert kwargs["exporter_name"] is None + paths = [d["path"] for d in result["drivers"]] + assert "client.power" in paths + assert result["cli_tree"]["name"] == "j" + json.dumps(result) + + @pytest.mark.anyio + async def test_empty_lease_name_raises(self, mock_config): + with pytest.raises(ValueError, match="non-empty"): + await describe_devices_async(mock_config, "") + mock_config.lease_async.assert_not_called() + + def test_blocking_wrapper(self, mock_config, mock_client_from_path): + result = describe_devices(mock_config, "existing-lease") + + kwargs = mock_config.lease_async.call_args.kwargs + assert kwargs["lease_name"] == "existing-lease" + assert result["cli_tree"]["name"] == "j" From f3fd4ca1b486bcb3d16d3e5ef30ed06739393c62 Mon Sep 17 00:00:00 2001 From: Kirk Brauer Date: Sat, 29 Aug 2026 18:31:33 -0400 Subject: [PATCH 03/11] feat(cli): add --devices to jmp describe lease Connects to the leased exporter via jumpstarter.client.introspect and appends the device tree to the description: a Devices table (driver path, client class, methods) and a Commands table listing every leaf of the j command tree with its help text, so the full runnable driver command surface is discoverable without entering a shell. With -o json/yaml the output becomes {lease, devices} where devices carries the drivers list and the complete cli_tree for machine consumers. Assisted-by: Claude:claude-fable-5 Signed-off-by: Kirk Brauer --- .../jumpstarter_cli/describe.py | 95 ++++++++++++++---- .../jumpstarter_cli/describe_test.py | 97 +++++++++++++++++++ 2 files changed, 172 insertions(+), 20 deletions(-) diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/describe.py b/python/packages/jumpstarter-cli/jumpstarter_cli/describe.py index e7e5c7db5..d5609f30d 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/describe.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/describe.py @@ -10,6 +10,8 @@ from pydantic import BaseModel, ConfigDict, Field from .login import relogin_client +from jumpstarter.client.grpc import Lease +from jumpstarter.client.introspect import describe_devices from jumpstarter.config.client import ClientConfigV1Alpha1 from jumpstarter.config.user import UserConfigV1Alpha1 @@ -57,25 +59,20 @@ def _condition_time(condition) -> datetime | None: def _print_conditions(conditions) -> None: - if not conditions: - click.echo("Conditions: ") - return - click.echo("Conditions:") - headers = ["Type", "Status", "Reason", "Message", "Last Transition Time"] - rows = [ + _print_table( + "Conditions", + ["Type", "Status", "Reason", "Message", "Last Transition Time"], [ - _format_value(condition.type), - _format_value(condition.status), - _format_value(condition.reason), - _format_value(condition.message), - _format_value(_condition_time(condition)), - ] - for condition in conditions - ] - widths = [max([len(header)] + [len(row[i]) for row in rows]) for i, header in enumerate(headers)] - dashes = ["-" * len(header) for header in headers] - for cells in [headers, dashes, *rows]: - click.echo(" " + " ".join(cell.ljust(width) for cell, width in zip(cells, widths, strict=True)).rstrip()) + [ + _format_value(condition.type), + _format_value(condition.status), + _format_value(condition.reason), + _format_value(condition.message), + _format_value(_condition_time(condition)), + ] + for condition in conditions + ], + ) @click.group(cls=AliasedGroup) @@ -135,20 +132,76 @@ def describe_exporter(config, name: str, output: OutputType): click.echo("Lease: ") +class LeaseDescription(BaseModel): + lease: Lease + devices: dict + + +def _walk_commands(tree: dict, path: list[str]) -> list[tuple[str, str]]: + commands = [] + for name, subtree in sorted((tree.get("subcommands") or {}).items()): + subpath = [*path, name] + if subtree.get("subcommands"): + commands.extend(_walk_commands(subtree, subpath)) + else: + help_text = (subtree.get("help") or "").strip().splitlines() + commands.append((" ".join(subpath), help_text[0] if help_text else "")) + return commands + + +def _print_table(label: str, headers: list[str], rows: list[list[str]]) -> None: + if not rows: + click.echo(f"{label}: ") + return + click.echo(f"{label}:") + widths = [max([len(header)] + [len(row[i]) for row in rows]) for i, header in enumerate(headers)] + dashes = ["-" * len(header) for header in headers] + for cells in [headers, dashes, *rows]: + click.echo(" " + " ".join(cell.ljust(width) for cell, width in zip(cells, widths, strict=True)).rstrip()) + + +def _print_devices(devices: dict) -> None: + _print_table( + "Devices", + ["Path", "Class", "Methods"], + [ + [ + ".".join(driver["driver_path"]) or "(root)", + driver["class"], + ", ".join(driver["methods"]), + ] + for driver in devices["drivers"] + ], + ) + commands = _walk_commands(devices["cli_tree"], ["j"]) if devices.get("cli_tree") else [] + _print_table("Commands", ["Command", "Description"], [[command, help] for command, help in commands]) + + @describe.command(name="lease") @opt_config(exporter=False) @click.argument("name") +@click.option( + "--devices", + "show_devices", + is_flag=True, + default=False, + help="Connect to the leased exporter and include its device tree and driver commands.", +) @opt_output @handle_exceptions_with_reauthentication(relogin_client) -def describe_lease(config, name: str, output: OutputType): +def describe_lease(config, name: str, show_devices: bool, output: OutputType): """ Show details of a specific lease """ lease = config.get_lease(name=name) + devices = describe_devices(config, name) if show_devices else None if output: - model_print(lease, output) + if devices is not None: + model_print(LeaseDescription(lease=lease, devices=devices), output) + else: + model_print(lease, output) return _print_fields( @@ -167,6 +220,8 @@ def describe_lease(config, name: str, output: OutputType): _print_mapping("Tags", lease.tags) _print_mapping("Context", lease.context) _print_conditions(lease.conditions) + if devices is not None: + _print_devices(devices) class ClientDescription(BaseModel): diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/describe_test.py b/python/packages/jumpstarter-cli/jumpstarter_cli/describe_test.py index 90507c249..6980951d8 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/describe_test.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/describe_test.py @@ -340,3 +340,100 @@ def test_desc_alias(self): ctx = MagicMock() ctx.fail = MagicMock() assert jmp.get_command(ctx, "desc") is describe + + +_DEVICES = { + "drivers": [ + { + "path": "client", + "driver_path": [], + "class": "jumpstarter_driver_composite.client.CompositeClient", + "description": None, + "methods": [], + }, + { + "path": "client.power", + "driver_path": ["power"], + "class": "jumpstarter_driver_power.client.PowerClient", + "description": None, + "methods": ["cycle", "off", "on"], + }, + ], + "cli_tree": { + "name": "j", + "help": "Generic composite device", + "params": [], + "subcommands": { + "power": { + "name": "power", + "help": "Power control", + "params": [], + "subcommands": { + "on": {"name": "on", "help": "Turn power on", "params": [], "subcommands": {}}, + "off": {"name": "off", "help": "Turn power off", "params": [], "subcommands": {}}, + }, + } + }, + }, +} + + +class TestDescribeLeaseDevices: + def setup_method(self): + self.runner = CliRunner() + + def test_pretty_output_devices(self): + config = MagicMock() + config.get_lease.return_value = _make_lease() + with ( + _patch_remote_config(config), + patch("jumpstarter_cli.describe.describe_devices", return_value=_DEVICES) as mock_devices, + ): + result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test", "--devices"]) + assert result.exit_code == 0, result.output + assert "Devices:" in result.output + assert "(root)" in result.output + assert "jumpstarter_driver_power.client.PowerClient" in result.output + assert "cycle, off, on" in result.output + assert "Commands:" in result.output + assert "j power on" in result.output + assert "Turn power on" in result.output + mock_devices.assert_called_once_with(config, "lease-1") + + def test_pretty_output_no_devices_flag(self): + config = MagicMock() + config.get_lease.return_value = _make_lease() + with ( + _patch_remote_config(config), + patch("jumpstarter_cli.describe.describe_devices", return_value=_DEVICES) as mock_devices, + ): + result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test"]) + assert result.exit_code == 0, result.output + assert "Devices:" not in result.output + mock_devices.assert_not_called() + + def test_json_output_devices(self): + config = MagicMock() + config.get_lease.return_value = _make_lease() + with ( + _patch_remote_config(config), + patch("jumpstarter_cli.describe.describe_devices", return_value=_DEVICES), + ): + result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test", "--devices", "-o", "json"]) + assert result.exit_code == 0, result.output + data = json.loads(result.output) + assert data["lease"]["name"] == "lease-1" + assert data["devices"]["drivers"][1]["class"] == "jumpstarter_driver_power.client.PowerClient" + assert data["devices"]["cli_tree"]["subcommands"]["power"]["subcommands"]["on"]["help"] == "Turn power on" + + def test_stub_root_cli_tree_none(self): + config = MagicMock() + config.get_lease.return_value = _make_lease() + devices = {"drivers": _DEVICES["drivers"], "cli_tree": None} + with ( + _patch_remote_config(config), + patch("jumpstarter_cli.describe.describe_devices", return_value=devices), + ): + result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test", "--devices"]) + assert result.exit_code == 0, result.output + assert "Commands: " in result.output From 0dfc27c7355e6cc8aa5cc0fcd9d640199a895d50 Mon Sep 17 00:00:00 2001 From: Kirk Brauer Date: Sat, 29 Aug 2026 22:17:11 -0400 Subject: [PATCH 04/11] fix(client): drive lease introspection from outside the event loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit describe_devices created a BlockingPortal inside the event loop and then introspected the driver client from that same thread. Driver clients are synchronous facades that dispatch through the portal, so every real connection raised "This method cannot be called from the event loop thread" — the unit tests missed it because their fake client never dispatches. Invert the ownership to match ClientConfigV1Alpha1.lease: the blocking caller owns the portal (its loop runs in its own thread) and introspects from the calling thread. describe_devices_async now runs that flow in a worker thread. Signed-off-by: Kirk Brauer --- .../jumpstarter/client/introspect.py | 66 ++++++++++++------- 1 file changed, 43 insertions(+), 23 deletions(-) diff --git a/python/packages/jumpstarter/jumpstarter/client/introspect.py b/python/packages/jumpstarter/jumpstarter/client/introspect.py index 6a4b17ea4..1ac3c4283 100644 --- a/python/packages/jumpstarter/jumpstarter/client/introspect.py +++ b/python/packages/jumpstarter/jumpstarter/client/introspect.py @@ -4,12 +4,13 @@ import inspect import logging -from contextlib import ExitStack +from contextlib import ExitStack, asynccontextmanager from datetime import timedelta from typing import TYPE_CHECKING, Any import click import click.core +from anyio import to_thread from anyio.from_thread import BlockingPortal, start_blocking_portal from .base import StubDriverClient @@ -240,12 +241,36 @@ def describe_client(client: Any) -> dict[str, Any]: } -async def describe_devices_async(config: ClientConfigV1Alpha1, lease_name: str) -> dict[str, Any]: +@asynccontextmanager +async def _connect_lease(config: ClientConfigV1Alpha1, lease_name: str, portal: BlockingPortal): + """Attach to an existing lease and yield its root driver client. + + Passing lease_name into lease_async attaches to that lease rather than + creating one, and leaves it unreleased on exit. + """ + async with config.lease_async( + selector=None, + exporter_name=None, + lease_name=lease_name, + duration=timedelta(minutes=30), + portal=portal, + ) as lease: + async with lease.serve_unix_async() as path: + with ExitStack() as stack: + async with client_from_path( + path, portal, stack, allow=lease.allow, unsafe=lease.unsafe + ) as client: + yield client + + +def describe_devices(config: ClientConfigV1Alpha1, lease_name: str) -> dict[str, Any]: """Attach to an existing lease and describe its driver clients and CLI tree. - Attaches to the lease named lease_name without creating a new lease and - without releasing it on exit, builds the root driver client, and returns - a plain-serializable dict with drivers and cli_tree keys. + Returns a plain-serializable dict with drivers and cli_tree keys. + + Driver clients are synchronous facades over a blocking portal, so the + portal has to run its event loop in its own thread and be driven from + this one — introspection therefore happens outside the loop. :raises ValueError: if lease_name is empty :raises jumpstarter.client.exceptions.LeaseError: if the lease has ended @@ -256,23 +281,18 @@ async def describe_devices_async(config: ClientConfigV1Alpha1, lease_name: str) if not lease_name: raise ValueError("lease_name must be a non-empty existing lease name") - async with BlockingPortal() as portal: - async with config.lease_async( - selector=None, - exporter_name=None, - lease_name=lease_name, - duration=timedelta(minutes=30), - portal=portal, - ) as lease: - async with lease.serve_unix_async() as path: - with ExitStack() as stack: - async with client_from_path( - path, portal, stack, allow=lease.allow, unsafe=lease.unsafe - ) as client: - return describe_client(client) + with start_blocking_portal() as portal: + with portal.wrap_async_context_manager(_connect_lease(config, lease_name, portal)) as client: + return describe_client(client) + +async def describe_devices_async(config: ClientConfigV1Alpha1, lease_name: str) -> dict[str, Any]: + """Async wrapper around describe_devices, run in a worker thread. -def describe_devices(config: ClientConfigV1Alpha1, lease_name: str) -> dict[str, Any]: - """Blocking convenience wrapper around describe_devices_async.""" - with start_blocking_portal() as portal: - return portal.call(describe_devices_async, config, lease_name) + The driver clients cannot be driven from an event loop thread, so the + whole attach-and-introspect flow runs off-loop. + """ + if not lease_name: + raise ValueError("lease_name must be a non-empty existing lease name") + + return await to_thread.run_sync(describe_devices, config, lease_name) From fb7b060e5b724b20b5caadbf16579adbea3d679d Mon Sep 17 00:00:00 2001 From: Kirk Brauer Date: Sat, 29 Aug 2026 23:57:09 -0400 Subject: [PATCH 05/11] feat(client): describe command parameters well enough to prompt for them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit walk_click_tree reported a parameter's name, type, help, required flag and default — not whether it is positional or an option, which flag spells it, whether it is a boolean flag, whether it repeats, or the values a choice accepts. A caller cannot build a command line, or ask a user for the values, without those. Also report Click's own type name: str() on types like Path renders an object repr, which is no use in a prompt. Signed-off-by: Kirk Brauer --- .../jumpstarter/client/introspect.py | 37 +++++++++++---- .../jumpstarter/client/introspect_test.py | 45 +++++++++++++++++++ 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/python/packages/jumpstarter/jumpstarter/client/introspect.py b/python/packages/jumpstarter/jumpstarter/client/introspect.py index 1ac3c4283..b457ea1fd 100644 --- a/python/packages/jumpstarter/jumpstarter/client/introspect.py +++ b/python/packages/jumpstarter/jumpstarter/client/introspect.py @@ -31,6 +31,33 @@ def _json_safe(value: Any) -> Any: return str(value) +def _describe_param(param: click.Parameter) -> dict[str, Any]: + """Describe one Click parameter well enough to prompt for it. + + Beyond name and help, a caller building a command line needs to know + whether the parameter is positional or an option (and which flag spells + it), whether it is a boolean flag that takes no value, whether it repeats, + and the values it accepts when it is a choice. + """ + described: dict[str, Any] = { + "name": param.name, + "kind": param.param_type_name, + # Click's own type name ("integer", "path", "choice"); str() on some + # types renders an object repr, which is no use in a prompt. + "type": getattr(param.type, "name", None) or str(param.type), + "help": getattr(param, "help", None), + "required": getattr(param, "required", False), + "default": _json_safe(param.default) if param.default is not None else None, + "opts": list(param.opts), + "is_flag": bool(getattr(param, "is_flag", False)), + "multiple": bool(getattr(param, "multiple", False)), + "nargs": param.nargs, + } + if isinstance(param.type, click.Choice): + described["choices"] = [str(choice) for choice in param.type.choices] + return described + + def walk_click_tree(cmd: click.core.BaseCommand, path: list[str] | None = None) -> dict[str, Any]: # ty: ignore[unresolved-attribute] """Recursively walk a Click command tree and return structured JSON. @@ -42,15 +69,7 @@ def walk_click_tree(cmd: click.core.BaseCommand, path: list[str] | None = None) "name": cmd.name, "help": cmd.help, "params": [ - { - "name": p.name, - "type": str(p.type), - "help": getattr(p, "help", None), - "required": getattr(p, "required", False), - "default": _json_safe(p.default) if p.default is not None else None, - } - for p in cmd.params - if not getattr(p, "hidden", False) and p.name != "help" + _describe_param(p) for p in cmd.params if not getattr(p, "hidden", False) and p.name != "help" ], } if isinstance(cmd, click.Group): diff --git a/python/packages/jumpstarter/jumpstarter/client/introspect_test.py b/python/packages/jumpstarter/jumpstarter/client/introspect_test.py index e5c5c4e32..52f9da13d 100644 --- a/python/packages/jumpstarter/jumpstarter/client/introspect_test.py +++ b/python/packages/jumpstarter/jumpstarter/client/introspect_test.py @@ -319,3 +319,48 @@ def test_blocking_wrapper(self, mock_config, mock_client_from_path): kwargs = mock_config.lease_async.call_args.kwargs assert kwargs["lease_name"] == "existing-lease" assert result["cli_tree"]["name"] == "j" + + +class TestParamDescription: + def test_describes_arguments_and_options(self): + @click.group() + def root(): + pass + + @root.command() + @click.argument("port", type=int) + @click.option("--address", help="Local address to bind") + @click.option("--verbose", is_flag=True) + @click.option("--mode", type=click.Choice(["fast", "slow"]), default="fast") + @click.option("--tag", multiple=True) + def forward(port, address, verbose, mode, tag): + """Forward a port.""" + + tree = walk_click_tree(root) + params = {p["name"]: p for p in tree["subcommands"]["forward"]["params"]} + + # A positional argument is spelled by name and an option by its flag, + # so a caller building a command line has to tell them apart. + assert params["port"]["kind"] == "argument" + assert params["port"]["required"] is True + assert params["port"]["type"] == "integer" + assert params["address"]["kind"] == "option" + assert params["address"]["opts"] == ["--address"] + assert params["address"]["help"] == "Local address to bind" + assert params["verbose"]["is_flag"] is True + assert params["mode"]["choices"] == ["fast", "slow"] + assert params["tag"]["multiple"] is True + + def test_type_name_is_readable(self): + @click.group() + def root(): + pass + + @root.command() + @click.argument("path", type=click.Path()) + def send(path): + pass + + params = walk_click_tree(root)["subcommands"]["send"]["params"] + # str(click.Path()) is an object repr, which is useless in a prompt. + assert params[0]["type"] == "path" From 5d9e79206f2e34e4c1fed88a1db0148932b82b46 Mon Sep 17 00:00:00 2001 From: Kirk Brauer Date: Mon, 31 Aug 2026 10:28:21 -0400 Subject: [PATCH 06/11] refactor(client): address review on the introspection move Four points from review: - Run list_drivers and get_driver_methods off the loop thread in the MCP tools, as explore() already does for client.cli(). They read attributes statically and so do not dispatch today, but a driver property that ever did would deadlock the loop. - Export describe_client, describe_devices and describe_devices_async from jumpstarter.client, since the point of the move was to make them callable from outside the MCP server. - Stop naming a 30 minute duration when attaching to an existing lease. Attaching by name never reaches Lease._create, and with selector None the "selector changed" branch cannot fire either, so nothing is ever sent to the controller; the number only suggested this call could extend a lease it cannot. - Import the introspection helpers by their real path in tools/connections.py. The jumpstarter_mcp.introspect shim is there for external callers, not for this package's own modules. The cli guard keeps checking the type rather than type(client).__dict__: a driver client may inherit its CLI instead of defining one, as QemuFlasherClient does from FlasherClientInterface, and a __dict__ check would drop the CLI tree for every such driver. It now catches a failing cli() so a broken one costs the CLI tree rather than the whole description. Both cases are covered by tests. Assisted-by: Claude Signed-off-by: Kirk Brauer --- .../jumpstarter_mcp/tools/commands.py | 7 +++-- .../jumpstarter_mcp/tools/connections.py | 2 +- .../jumpstarter/client/__init__.py | 13 +++++++++- .../jumpstarter/client/introspect.py | 16 ++++++++++-- .../jumpstarter/client/introspect_test.py | 26 +++++++++++++++++++ 5 files changed, 58 insertions(+), 6 deletions(-) diff --git a/python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/commands.py b/python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/commands.py index dc759f46d..49bd8a273 100644 --- a/python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/commands.py +++ b/python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/commands.py @@ -120,7 +120,10 @@ async def drivers( ) -> list[dict]: """List all drivers in the client tree.""" conn = manager.get_connection(connection_id) - return list_drivers(conn.client) + # Off the loop thread, as explore() does. These walk a sync client facade + # over a blocking portal; they read attributes statically today and so do + # not dispatch, but a driver property that ever did would deadlock the loop. + return await anyio.to_thread.run_sync(list_drivers, conn.client) async def driver_methods( @@ -130,4 +133,4 @@ async def driver_methods( ) -> dict: """Inspect methods on a specific driver.""" conn = manager.get_connection(connection_id) - return get_driver_methods(conn.client, driver_path) + return await anyio.to_thread.run_sync(get_driver_methods, conn.client, driver_path) diff --git a/python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/connections.py b/python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/connections.py index 6487ed8d0..85d889b7e 100644 --- a/python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/connections.py +++ b/python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/connections.py @@ -8,8 +8,8 @@ import anyio.to_thread # noqa: F401 from jumpstarter_mcp.connections import ConnectionManager -from jumpstarter_mcp.introspect import list_drivers, walk_click_tree +from jumpstarter.client.introspect import list_drivers, walk_click_tree from jumpstarter.config.client import ClientConfigV1Alpha1 logger = logging.getLogger(__name__) diff --git a/python/packages/jumpstarter/jumpstarter/client/__init__.py b/python/packages/jumpstarter/jumpstarter/client/__init__.py index 62dbf689c..b8df2a654 100644 --- a/python/packages/jumpstarter/jumpstarter/client/__init__.py +++ b/python/packages/jumpstarter/jumpstarter/client/__init__.py @@ -1,6 +1,17 @@ from .base import DriverClient from .client import client_from_path from .flasher import FlasherClient, FlasherClientInterface +from .introspect import describe_client, describe_devices, describe_devices_async from .lease import DirectLease, Lease -__all__ = ["DriverClient", "DirectLease", "FlasherClient", "FlasherClientInterface", "client_from_path", "Lease"] +__all__ = [ + "DriverClient", + "DirectLease", + "FlasherClient", + "FlasherClientInterface", + "client_from_path", + "Lease", + "describe_client", + "describe_devices", + "describe_devices_async", +] diff --git a/python/packages/jumpstarter/jumpstarter/client/introspect.py b/python/packages/jumpstarter/jumpstarter/client/introspect.py index b457ea1fd..d7f25d1e9 100644 --- a/python/packages/jumpstarter/jumpstarter/client/introspect.py +++ b/python/packages/jumpstarter/jumpstarter/client/introspect.py @@ -252,8 +252,16 @@ def describe_client(client: Any) -> dict[str, Any]: (including when the root client itself is a stub). """ cli_tree = None + # Inherited counts: QemuFlasherClient, for one, defines no cli of its own and + # gets a real one from FlasherClientInterface, so looking only at + # type(client).__dict__ would drop the CLI of every such driver. if not isinstance(client, StubDriverClient) and getattr(type(client), "cli", None) is not None: - cli_tree = walk_click_tree(client.cli()) + try: + cli_tree = walk_click_tree(client.cli()) + except Exception: + # A driver whose cli() is broken should cost us its CLI tree, not the + # whole description — the driver listing below is still worth having. + logger.warning("could not build the CLI tree for %s", type(client).__name__, exc_info=True) return { "drivers": list_drivers(client), "cli_tree": cli_tree, @@ -271,7 +279,11 @@ async def _connect_lease(config: ClientConfigV1Alpha1, lease_name: str, portal: selector=None, exporter_name=None, lease_name=lease_name, - duration=timedelta(minutes=30), + # Attaching by name never reaches Lease._create, and with selector None + # the "selector changed, make a new one" branch cannot fire either, so + # no duration is ever sent to the controller. Naming 30 minutes here + # only suggested this call could extend a lease that it cannot. + duration=timedelta(0), portal=portal, ) as lease: async with lease.serve_unix_async() as path: diff --git a/python/packages/jumpstarter/jumpstarter/client/introspect_test.py b/python/packages/jumpstarter/jumpstarter/client/introspect_test.py index 52f9da13d..5ea8cb4a7 100644 --- a/python/packages/jumpstarter/jumpstarter/client/introspect_test.py +++ b/python/packages/jumpstarter/jumpstarter/client/introspect_test.py @@ -212,6 +212,32 @@ def test_cli_tree_none_without_cli(self): assert result["cli_tree"] is None assert result["drivers"][0]["path"] == "client" + def test_an_inherited_cli_still_counts(self): + """A driver client is free to inherit its CLI rather than define one. + + QemuFlasherClient is a real example: it has no cli of its own and takes + one from FlasherClientInterface. Looking only at type(client).__dict__ + would drop the CLI tree for every such driver. + """ + + class InheritsCli(FakeCompositeClient): + pass + + assert "cli" not in InheritsCli.__dict__ + result = describe_client(InheritsCli()) + assert result["cli_tree"]["name"] == "j" + + def test_a_broken_cli_costs_only_the_cli_tree(self): + """The driver listing is worth having even when cli() raises.""" + + class BrokenCli(FakeCompositeClient): + def cli(self): + raise RuntimeError("this driver's cli is broken") + + result = describe_client(BrokenCli()) + assert result["cli_tree"] is None + assert [d["path"] for d in result["drivers"]] != [] + def test_composite_client_e2e(self): from jumpstarter_driver_composite.driver import Composite from jumpstarter_driver_power.driver import MockPower From 6ab81852260a678b416cc0cc0b83a47963044187 Mon Sep 17 00:00:00 2001 From: Kirk Brauer Date: Sat, 5 Sep 2026 16:06:27 -0400 Subject: [PATCH 07/11] feat(cli): describe lease driver trees with --drivers Use driver_tree for structured output and Drivers for the human-readable heading. These are software driver clients, not a physical device inventory. Keep the initial library helper names as compatibility aliases without introducing a --devices CLI alias or changing the existing exporter device report. Assisted-by: Pi:gpt-6-astra Signed-off-by: Kirk Brauer --- .../guides/setup/distributed-mode.md | 18 +++-- .../jumpstarter_cli/describe.py | 30 ++++----- .../jumpstarter_cli/describe_test.py | 66 ++++++++++--------- .../jumpstarter/client/introspect.py | 14 ++-- .../jumpstarter/client/introspect_test.py | 18 +++-- 5 files changed, 84 insertions(+), 62 deletions(-) diff --git a/docs/source/getting-started/guides/setup/distributed-mode.md b/docs/source/getting-started/guides/setup/distributed-mode.md index 41a7c3ebe..1af214d62 100644 --- a/docs/source/getting-started/guides/setup/distributed-mode.md +++ b/docs/source/getting-started/guides/setup/distributed-mode.md @@ -136,23 +136,27 @@ Conditions: Ready True Ready An exporter has been acquired for the client 2026-08-31 14:04:36 UTC ``` -For a lease you hold, add `--devices` to connect to its exporter and discover +For a lease you hold, add `--drivers` to connect to its exporter and discover its drivers and runnable `j` commands: ```console -$ jmp describe lease 01a05822-e378-71cc-a98c-a216ad4a9432 --client hello --devices -$ jmp describe lease 01a05822-e378-71cc-a98c-a216ad4a9432 --client hello --devices -o json +$ jmp describe lease 01a05822-e378-71cc-a98c-a216ad4a9432 --client hello --drivers +$ jmp describe lease 01a05822-e378-71cc-a98c-a216ad4a9432 --client hello --drivers -o json ``` -The human-readable description adds **Devices** and **Commands** tables. With -`-o json` or `-o yaml`, the result is `{lease, devices}`: `lease` contains the -usual lease metadata, while `devices` contains a `drivers` list and the recursive +The human-readable description adds **Drivers** and **Commands** tables. With +`-o json` or `-o yaml`, the result is `{lease, driver_tree}`: `lease` contains the +usual lease metadata, while `driver_tree` contains a `drivers` list and the recursive `cli_tree`, including command help and parameters. This is discovery only; it does not execute the listed driver commands, create a lease, or release your existing lease when it finishes. It does require a connection to the exporter -and uses the selected client's driver-access settings. Without `--devices`, +and uses the selected client's driver-access settings. Without `--drivers`, the existing metadata-only behavior and output shape are unchanged. +Here, **drivers** means the software driver clients exposed by the lease session, +not an inventory of physical devices attached to the exporter. The exporter's +existing device report is unchanged by this command. + Describing a client reads the local configuration rather than the cluster, so it works without a connection and reports whether the client's token is still valid: diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/describe.py b/python/packages/jumpstarter-cli/jumpstarter_cli/describe.py index 7c1cd35a3..755295f24 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/describe.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/describe.py @@ -11,7 +11,7 @@ from .login import relogin_client from jumpstarter.client.grpc import Lease -from jumpstarter.client.introspect import describe_devices +from jumpstarter.client.introspect import describe_drivers from jumpstarter.config.client import ClientConfigV1Alpha1 from jumpstarter.config.user import UserConfigV1Alpha1 @@ -148,7 +148,7 @@ def describe_exporter(config, name: str, output: OutputType): class LeaseDescription(BaseModel): lease: Lease - devices: dict + driver_tree: dict def _walk_commands(tree: dict, path: list[str]) -> list[tuple[str, str]]: @@ -174,9 +174,9 @@ def _print_table(label: str, headers: list[str], rows: list[list[str]]) -> None: click.echo(" " + " ".join(cell.ljust(width) for cell, width in zip(cells, widths, strict=True)).rstrip()) -def _print_devices(devices: dict) -> None: +def _print_drivers(driver_tree: dict) -> None: _print_table( - "Devices", + "Drivers", ["Path", "Class", "Methods"], [ [ @@ -184,10 +184,10 @@ def _print_devices(devices: dict) -> None: driver["class"], ", ".join(driver["methods"]), ] - for driver in devices["drivers"] + for driver in driver_tree["drivers"] ], ) - commands = _walk_commands(devices["cli_tree"], ["j"]) if devices.get("cli_tree") else [] + commands = _walk_commands(driver_tree["cli_tree"], ["j"]) if driver_tree.get("cli_tree") else [] _print_table("Commands", ["Command", "Description"], [[command, help] for command, help in commands]) @@ -195,25 +195,25 @@ def _print_devices(devices: dict) -> None: @opt_config(exporter=False) @click.argument("name") @click.option( - "--devices", - "show_devices", + "--drivers", + "show_drivers", is_flag=True, default=False, - help="Connect to the leased exporter and include its device tree and driver commands.", + help="Connect to the leased exporter and include its driver tree and driver commands.", ) @opt_output @handle_exceptions_with_reauthentication(relogin_client) -def describe_lease(config, name: str, show_devices: bool, output: OutputType): +def describe_lease(config, name: str, show_drivers: bool, output: OutputType): """ Show details of a specific lease """ lease = config.get_lease(name=name) - devices = describe_devices(config, name) if show_devices else None + driver_tree = describe_drivers(config, name) if show_drivers else None if output: - if devices is not None: - model_print(LeaseDescription(lease=lease, devices=devices), output) + if driver_tree is not None: + model_print(LeaseDescription(lease=lease, driver_tree=driver_tree), output) else: model_print(lease, output) return @@ -234,8 +234,8 @@ def describe_lease(config, name: str, show_devices: bool, output: OutputType): _print_mapping("Tags", lease.tags) _print_mapping("Context", lease.context) _print_conditions(lease.conditions) - if devices is not None: - _print_devices(devices) + if driver_tree is not None: + _print_drivers(driver_tree) class ClientDescription(BaseModel): diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/describe_test.py b/python/packages/jumpstarter-cli/jumpstarter_cli/describe_test.py index 7bb3ba67f..02075cb93 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/describe_test.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/describe_test.py @@ -362,7 +362,7 @@ def test_desc_alias(self): assert jmp.get_command(ctx, "desc") is describe -_DEVICES = { +_DRIVER_TREE = { "drivers": [ { "path": "client", @@ -398,101 +398,107 @@ def test_desc_alias(self): } -class TestDescribeLeaseDevices: +class TestDescribeLeaseDrivers: def setup_method(self): self.runner = CliRunner() - def test_pretty_output_devices(self): + def test_devices_is_not_an_alias_for_driver_introspection(self): + result = self.runner.invoke(describe, ["lease", "lease-1", "--devices"]) + assert result.exit_code == 2 + assert "No such option: --devices" in result.output + + def test_pretty_output_drivers(self): config = MagicMock() config.get_lease.return_value = _make_lease() with ( _patch_remote_config(config), - patch("jumpstarter_cli.describe.describe_devices", return_value=_DEVICES) as mock_devices, + patch("jumpstarter_cli.describe.describe_drivers", return_value=_DRIVER_TREE) as mock_drivers, ): - result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test", "--devices"]) + result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test", "--drivers"]) assert result.exit_code == 0, result.output - assert "Devices:" in result.output + assert "Drivers:" in result.output assert "(root)" in result.output assert "jumpstarter_driver_power.client.PowerClient" in result.output assert "cycle, off, on" in result.output assert "Commands:" in result.output assert "j power on" in result.output assert "Turn power on" in result.output - mock_devices.assert_called_once_with(config, "lease-1") + mock_drivers.assert_called_once_with(config, "lease-1") - def test_pretty_output_no_devices_flag(self): + def test_pretty_output_no_drivers_flag(self): config = MagicMock() config.get_lease.return_value = _make_lease() with ( _patch_remote_config(config), - patch("jumpstarter_cli.describe.describe_devices", return_value=_DEVICES) as mock_devices, + patch("jumpstarter_cli.describe.describe_drivers", return_value=_DRIVER_TREE) as mock_drivers, ): result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test"]) assert result.exit_code == 0, result.output - assert "Devices:" not in result.output - mock_devices.assert_not_called() + assert "Drivers:" not in result.output + mock_drivers.assert_not_called() - def test_json_output_devices(self): + def test_json_output_drivers(self): config = MagicMock() config.get_lease.return_value = _make_lease() with ( _patch_remote_config(config), - patch("jumpstarter_cli.describe.describe_devices", return_value=_DEVICES), + patch("jumpstarter_cli.describe.describe_drivers", return_value=_DRIVER_TREE), ): - result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test", "--devices", "-o", "json"]) + result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test", "--drivers", "-o", "json"]) assert result.exit_code == 0, result.output data = json.loads(result.output) assert data["lease"]["name"] == "lease-1" - assert data["devices"]["drivers"][1]["class"] == "jumpstarter_driver_power.client.PowerClient" - assert data["devices"]["cli_tree"]["subcommands"]["power"]["subcommands"]["on"]["help"] == "Turn power on" + assert data["driver_tree"]["drivers"][1]["class"] == "jumpstarter_driver_power.client.PowerClient" + assert data["driver_tree"]["cli_tree"]["subcommands"]["power"]["subcommands"]["on"]["help"] == "Turn power on" - def test_yaml_output_devices(self): + def test_yaml_output_drivers(self): config = MagicMock() config.get_lease.return_value = _make_lease() with ( _patch_remote_config(config), - patch("jumpstarter_cli.describe.describe_devices", return_value=_DEVICES), + patch("jumpstarter_cli.describe.describe_drivers", return_value=_DRIVER_TREE), ): - result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test", "--devices", "-o", "yaml"]) + result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test", "--drivers", "-o", "yaml"]) assert result.exit_code == 0, result.output data = yaml.safe_load(result.output) assert data["lease"]["name"] == "lease-1" - assert data["devices"] == _DEVICES + assert data["driver_tree"] == _DRIVER_TREE + assert "devices" not in data - def test_json_without_devices_preserves_the_lease_shape(self): + def test_json_without_drivers_preserves_the_lease_shape(self): config = MagicMock() config.get_lease.return_value = _make_lease() with ( _patch_remote_config(config), - patch("jumpstarter_cli.describe.describe_devices") as mock_devices, + patch("jumpstarter_cli.describe.describe_drivers") as mock_drivers, ): result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test", "-o", "json"]) assert result.exit_code == 0, result.output data = json.loads(result.output) assert data["name"] == "lease-1" assert "lease" not in data - assert "devices" not in data - mock_devices.assert_not_called() + assert "driver_tree" not in data + mock_drivers.assert_not_called() - def test_device_connection_failure_is_reported(self): + def test_driver_connection_failure_is_reported(self): config = MagicMock() config.get_lease.return_value = _make_lease() with ( _patch_remote_config(config), - patch("jumpstarter_cli.describe.describe_devices", side_effect=ConnectionError("exporter unreachable")), + patch("jumpstarter_cli.describe.describe_drivers", side_effect=ConnectionError("exporter unreachable")), ): - result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test", "--devices", "-o", "json"]) + result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test", "--drivers", "-o", "json"]) assert result.exit_code != 0 assert "exporter unreachable" in result.output def test_stub_root_cli_tree_none(self): config = MagicMock() config.get_lease.return_value = _make_lease() - devices = {"drivers": _DEVICES["drivers"], "cli_tree": None} + driver_tree = {"drivers": _DRIVER_TREE["drivers"], "cli_tree": None} with ( _patch_remote_config(config), - patch("jumpstarter_cli.describe.describe_devices", return_value=devices), + patch("jumpstarter_cli.describe.describe_drivers", return_value=driver_tree), ): - result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test", "--devices"]) + result = self.runner.invoke(describe, ["lease", "lease-1", "--client", "test", "--drivers"]) assert result.exit_code == 0, result.output assert "Commands: " in result.output diff --git a/python/packages/jumpstarter/jumpstarter/client/introspect.py b/python/packages/jumpstarter/jumpstarter/client/introspect.py index 6a4b17ea4..a3786f7e9 100644 --- a/python/packages/jumpstarter/jumpstarter/client/introspect.py +++ b/python/packages/jumpstarter/jumpstarter/client/introspect.py @@ -240,7 +240,7 @@ def describe_client(client: Any) -> dict[str, Any]: } -async def describe_devices_async(config: ClientConfigV1Alpha1, lease_name: str) -> dict[str, Any]: +async def describe_drivers_async(config: ClientConfigV1Alpha1, lease_name: str) -> dict[str, Any]: """Attach to an existing lease and describe its driver clients and CLI tree. Attaches to the lease named lease_name without creating a new lease and @@ -272,7 +272,13 @@ async def describe_devices_async(config: ClientConfigV1Alpha1, lease_name: str) return describe_client(client) -def describe_devices(config: ClientConfigV1Alpha1, lease_name: str) -> dict[str, Any]: - """Blocking convenience wrapper around describe_devices_async.""" +def describe_drivers(config: ClientConfigV1Alpha1, lease_name: str) -> dict[str, Any]: + """Blocking convenience wrapper around describe_drivers_async.""" with start_blocking_portal() as portal: - return portal.call(describe_devices_async, config, lease_name) + return portal.call(describe_drivers_async, config, lease_name) + + +# Compatibility for consumers of the initial introspection-library branch. +# These describe driver clients, not a physical device inventory. +describe_devices_async = describe_drivers_async +describe_devices = describe_drivers diff --git a/python/packages/jumpstarter/jumpstarter/client/introspect_test.py b/python/packages/jumpstarter/jumpstarter/client/introspect_test.py index e5c5c4e32..086a9bbe8 100644 --- a/python/packages/jumpstarter/jumpstarter/client/introspect_test.py +++ b/python/packages/jumpstarter/jumpstarter/client/introspect_test.py @@ -8,8 +8,8 @@ from jumpstarter.client.introspect import ( _get_public_method_names, describe_client, - describe_devices, - describe_devices_async, + describe_drivers, + describe_drivers_async, get_driver_methods, list_drivers, walk_click_tree, @@ -264,7 +264,13 @@ async def test_stub_client_represented(self): assert result["drivers"][0]["class"].endswith("StubDriverClient") -class TestDescribeDevices: +class TestDescribeDrivers: + def test_legacy_names_remain_compatible(self): + from jumpstarter.client.introspect import describe_devices, describe_devices_async + + assert describe_devices is describe_drivers + assert describe_devices_async is describe_drivers_async + @pytest.fixture() def mock_config(self): fake_lease = MagicMock() @@ -296,7 +302,7 @@ async def fake_client_from_path(path, portal, stack, allow, unsafe): @pytest.mark.anyio async def test_attaches_to_named_lease(self, mock_config, mock_client_from_path): - result = await describe_devices_async(mock_config, "existing-lease") + result = await describe_drivers_async(mock_config, "existing-lease") kwargs = mock_config.lease_async.call_args.kwargs assert kwargs["lease_name"] == "existing-lease" @@ -310,11 +316,11 @@ async def test_attaches_to_named_lease(self, mock_config, mock_client_from_path) @pytest.mark.anyio async def test_empty_lease_name_raises(self, mock_config): with pytest.raises(ValueError, match="non-empty"): - await describe_devices_async(mock_config, "") + await describe_drivers_async(mock_config, "") mock_config.lease_async.assert_not_called() def test_blocking_wrapper(self, mock_config, mock_client_from_path): - result = describe_devices(mock_config, "existing-lease") + result = describe_drivers(mock_config, "existing-lease") kwargs = mock_config.lease_async.call_args.kwargs assert kwargs["lease_name"] == "existing-lease" From 1dd009a53f862b034cea0ef448a6e87fa591e642 Mon Sep 17 00:00:00 2001 From: Kirk Brauer Date: Sat, 5 Sep 2026 16:12:47 -0400 Subject: [PATCH 08/11] test: cover introspection review fixes across MCP and client APIs Exercise MCP discovery with a live blocking portal so synchronous attribute access fails if it moves back onto the event loop. Verify the package-level public exports and the neutral duration placeholder used only when attaching to an existing lease. Assisted-by: Pi:gpt-6-astra Signed-off-by: Kirk Brauer --- .../jumpstarter_mcp/tools/commands_test.py | 61 +++++++++++++++++++ .../jumpstarter/client/introspect_test.py | 16 +++++ 2 files changed, 77 insertions(+) create mode 100644 python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/commands_test.py diff --git a/python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/commands_test.py b/python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/commands_test.py new file mode 100644 index 000000000..361c6f57c --- /dev/null +++ b/python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/commands_test.py @@ -0,0 +1,61 @@ +"""Introspection must drive synchronous driver facades outside the event loop.""" + +from types import SimpleNamespace +from unittest.mock import MagicMock + +import click +import pytest +from anyio import fail_after +from anyio.from_thread import BlockingPortal + +from jumpstarter_mcp.tools.commands import driver_methods, drivers, explore + + +class LeafClient: + children: dict = {} + + def on(self): + """Turn power on.""" + + +class PortalClient: + """Model driver attributes/CLI builders that dispatch through a live portal. + + Calling these on the event-loop thread raises immediately, just like a + synchronous driver facade. A static fake would miss that regression. + """ + + def __init__(self, portal): + self.portal = portal + + @property + def description(self): + return self.portal.call(lambda: "A remotely described driver") + + @property + def children(self): + return self.portal.call(lambda: {"power": LeafClient()}) + + def cli(self): + self.portal.call(lambda: None) + return click.Group("j", commands={"on": click.Command("on")}) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("tool", ["drivers", "driver_methods", "explore"]) +async def test_introspection_dispatches_off_the_loop(tool): + with fail_after(5): + async with BlockingPortal() as portal: + manager = MagicMock() + manager.get_connection.return_value = SimpleNamespace(client=PortalClient(portal)) + if tool == "drivers": + result = await drivers(manager, "connection-1") + assert result[0]["description"] == "A remotely described driver" + assert result[1]["driver_path"] == ["power"] + elif tool == "driver_methods": + result = await driver_methods(manager, "connection-1", ["power"]) + assert [method["name"] for method in result["methods"]] == ["on"] + else: + result = await explore(manager, "connection-1") + assert "on" in result["subcommands"] + manager.get_connection.assert_called_once_with("connection-1") diff --git a/python/packages/jumpstarter/jumpstarter/client/introspect_test.py b/python/packages/jumpstarter/jumpstarter/client/introspect_test.py index 5ea8cb4a7..c7550c76c 100644 --- a/python/packages/jumpstarter/jumpstarter/client/introspect_test.py +++ b/python/packages/jumpstarter/jumpstarter/client/introspect_test.py @@ -1,5 +1,6 @@ import json from contextlib import asynccontextmanager +from datetime import timedelta from unittest.mock import MagicMock, patch import click @@ -16,6 +17,18 @@ ) +def test_description_helpers_are_public_package_exports(): + import jumpstarter.client as client + + for name, helper in ( + ("describe_client", describe_client), + ("describe_devices", describe_devices), + ("describe_devices_async", describe_devices_async), + ): + assert name in client.__all__ + assert getattr(client, name) is helper + + class FakePowerClient: children: dict = {} @@ -328,6 +341,9 @@ async def test_attaches_to_named_lease(self, mock_config, mock_client_from_path) assert kwargs["lease_name"] == "existing-lease" assert kwargs["selector"] is None assert kwargs["exporter_name"] is None + # lease_async requires a duration argument, but attaching by name never + # sends it to the controller. Keep the placeholder explicitly neutral. + assert kwargs["duration"] == timedelta(0) paths = [d["path"] for d in result["drivers"]] assert "client.power" in paths assert result["cli_tree"]["name"] == "j" From 080009ebce2a43418174bbbf8c6800091d60cf51 Mon Sep 17 00:00:00 2001 From: Kirk Brauer Date: Sat, 5 Sep 2026 16:36:25 -0400 Subject: [PATCH 09/11] fix(client): retain declared help inputs and valid root call examples Only exclude hidden Click parameters: automatic help is not in cmd.params, while explicit inputs named help must remain discoverable. Build root method examples against client directly instead of emitting a doubled dot. Cover argument/option help inputs, automatic help exclusion, and parseable root and nested call examples. Assisted-by: Pi:gpt-6-astra Signed-off-by: Kirk Brauer --- .../jumpstarter/client/introspect.py | 5 ++-- .../jumpstarter/client/introspect_test.py | 28 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/python/packages/jumpstarter/jumpstarter/client/introspect.py b/python/packages/jumpstarter/jumpstarter/client/introspect.py index d7f25d1e9..555b1825a 100644 --- a/python/packages/jumpstarter/jumpstarter/client/introspect.py +++ b/python/packages/jumpstarter/jumpstarter/client/introspect.py @@ -69,7 +69,7 @@ def walk_click_tree(cmd: click.core.BaseCommand, path: list[str] | None = None) "name": cmd.name, "help": cmd.help, "params": [ - _describe_param(p) for p in cmd.params if not getattr(p, "hidden", False) and p.name != "help" + _describe_param(p) for p in cmd.params if not getattr(p, "hidden", False) ], } if isinstance(cmd, click.Group): @@ -189,7 +189,8 @@ def _inspect_method(name: str, method: Any, driver_path: list[str]) -> dict[str, attr_path = ".".join(driver_path) call_args = ", ".join(f"{p['name']}=..." for p in params) - method_call = f"client.{attr_path}.{name}({call_args})" + receiver = f"client.{attr_path}" if attr_path else "client" + method_call = f"{receiver}.{name}({call_args})" return { "name": name, diff --git a/python/packages/jumpstarter/jumpstarter/client/introspect_test.py b/python/packages/jumpstarter/jumpstarter/client/introspect_test.py index c7550c76c..c474a832c 100644 --- a/python/packages/jumpstarter/jumpstarter/client/introspect_test.py +++ b/python/packages/jumpstarter/jumpstarter/client/introspect_test.py @@ -1,3 +1,4 @@ +import ast import json from contextlib import asynccontextmanager from datetime import timedelta @@ -125,6 +126,25 @@ def sub2(count): assert result["subcommands"]["sub2"]["params"][0]["name"] == "count" assert result["subcommands"]["sub2"]["params"][0]["default"] == 5 + @pytest.mark.parametrize("parameter", [ + click.Argument(["help"], required=True), + click.Option(["--help"], required=True, help="An explicitly declared input"), + ]) + def test_declared_parameter_named_help_is_preserved(self, parameter): + command = click.Command("cmd", params=[parameter], add_help_option=False) + params = walk_click_tree(command)["params"] + assert len(params) == 1 + assert params[0]["name"] == "help" + assert params[0]["kind"] == parameter.param_type_name + assert params[0]["required"] is True + assert params[0]["opts"] == parameter.opts + + def test_automatic_help_is_not_a_declared_parameter(self): + command = click.Command("cmd") + with click.Context(command) as context: + assert command.get_help_option(context) is not None + assert walk_click_tree(command)["params"] == [] + def test_hidden_params_excluded(self): @click.command("cmd") @click.option("--visible", help="shown") @@ -190,6 +210,14 @@ def test_call_example_uses_dot_notation(self): cycle = next(m for m in result["methods"] if m["name"] == "cycle") assert "client.power.cycle(" in cycle["call_example"] assert 'children["power"]' not in cycle["call_example"] + ast.parse(cycle["call_example"]) + + def test_root_driver_example_is_valid_python(self): + result = get_driver_methods(FakePowerClient(), []) + cycle = next(m for m in result["methods"] if m["name"] == "cycle") + assert "client.cycle(wait=...)" in cycle["call_example"] + assert "client.." not in cycle["call_example"] + ast.parse(cycle["call_example"]) def test_invalid_path_raises(self): client = FakeCompositeClient() From b51f524937f77a5ed8ea9b4f5ac2470e37da18ee Mon Sep 17 00:00:00 2001 From: Kirk Brauer Date: Sat, 5 Sep 2026 18:58:28 -0400 Subject: [PATCH 10/11] fix(client): preserve structured mapping defaults in introspection Recursively coerce Mapping values before the string fallback, including string keys for JSON objects. Cover ordinary and read-only mappings, nested sequences, non-string keys, and non-JSON leaves through Click parameter descriptions. Assisted-by: Pi:gpt-6-astra Signed-off-by: Kirk Brauer --- .../jumpstarter/client/introspect.py | 3 +++ .../jumpstarter/client/introspect_test.py | 17 +++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/python/packages/jumpstarter/jumpstarter/client/introspect.py b/python/packages/jumpstarter/jumpstarter/client/introspect.py index 555b1825a..33f7f9130 100644 --- a/python/packages/jumpstarter/jumpstarter/client/introspect.py +++ b/python/packages/jumpstarter/jumpstarter/client/introspect.py @@ -4,6 +4,7 @@ import inspect import logging +from collections.abc import Mapping from contextlib import ExitStack, asynccontextmanager from datetime import timedelta from typing import TYPE_CHECKING, Any @@ -28,6 +29,8 @@ def _json_safe(value: Any) -> Any: return value if isinstance(value, (list, tuple)): return [_json_safe(v) for v in value] + if isinstance(value, Mapping): + return {str(key): _json_safe(item) for key, item in value.items()} return str(value) diff --git a/python/packages/jumpstarter/jumpstarter/client/introspect_test.py b/python/packages/jumpstarter/jumpstarter/client/introspect_test.py index c474a832c..0e7a410e8 100644 --- a/python/packages/jumpstarter/jumpstarter/client/introspect_test.py +++ b/python/packages/jumpstarter/jumpstarter/client/introspect_test.py @@ -2,6 +2,8 @@ import json from contextlib import asynccontextmanager from datetime import timedelta +from pathlib import Path +from types import MappingProxyType from unittest.mock import MagicMock, patch import click @@ -392,6 +394,21 @@ def test_blocking_wrapper(self, mock_config, mock_client_from_path): class TestParamDescription: + @pytest.mark.parametrize("mapping_type", [dict, MappingProxyType]) + def test_mapping_defaults_remain_structured(self, mapping_type): + """Preserve nested mappings while coercing keys and non-JSON leaves.""" + default = mapping_type({ + "nested": mapping_type({1: (Path("image.bin"), None, True)}), + "items": [mapping_type({"count": 2})], + }) + command = click.Command("cmd", params=[click.Option(["--config"], default=default)]) + tree = walk_click_tree(command) + assert tree["params"][0]["default"] == { + "nested": {"1": ["image.bin", None, True]}, + "items": [{"count": 2}], + } + assert json.loads(json.dumps(tree)) == tree + def test_describes_arguments_and_options(self): @click.group() def root(): From cf391f2f6ac4495312d5f7dab0256356e54dace3 Mon Sep 17 00:00:00 2001 From: Kirk Brauer Date: Sat, 5 Sep 2026 20:30:58 -0400 Subject: [PATCH 11/11] fix(client): reject colliding JSON mapping keys in defaults Reject ambiguous stringified keys rather than silently replacing a default value. Cover both key orders and collisions inside nested read-only mappings. Assisted-by: Pi:gpt-6-astra Signed-off-by: Kirk Brauer --- .../jumpstarter/jumpstarter/client/introspect.py | 8 +++++++- .../jumpstarter/jumpstarter/client/introspect_test.py | 11 +++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/python/packages/jumpstarter/jumpstarter/client/introspect.py b/python/packages/jumpstarter/jumpstarter/client/introspect.py index 33f7f9130..68beec77a 100644 --- a/python/packages/jumpstarter/jumpstarter/client/introspect.py +++ b/python/packages/jumpstarter/jumpstarter/client/introspect.py @@ -30,7 +30,13 @@ def _json_safe(value: Any) -> Any: if isinstance(value, (list, tuple)): return [_json_safe(v) for v in value] if isinstance(value, Mapping): - return {str(key): _json_safe(item) for key, item in value.items()} + result: dict[str, Any] = {} + for key, item in value.items(): + json_key = str(key) + if json_key in result: + raise ValueError(f"Mapping keys collide after stringification: {json_key!r}") + result[json_key] = _json_safe(item) + return result return str(value) diff --git a/python/packages/jumpstarter/jumpstarter/client/introspect_test.py b/python/packages/jumpstarter/jumpstarter/client/introspect_test.py index 0e7a410e8..16a6c7577 100644 --- a/python/packages/jumpstarter/jumpstarter/client/introspect_test.py +++ b/python/packages/jumpstarter/jumpstarter/client/introspect_test.py @@ -409,6 +409,17 @@ def test_mapping_defaults_remain_structured(self, mapping_type): } assert json.loads(json.dumps(tree)) == tree + @pytest.mark.parametrize("default", [ + {1: "numeric", "1": "string"}, + {"1": "string", 1: "numeric"}, + {"nested": [MappingProxyType({1: "numeric", "1": "string"})]}, + ]) + def test_mapping_key_collisions_are_rejected(self, default): + """Never report a default with values lost to stringified key collisions.""" + command = click.Command("cmd", params=[click.Option(["--config"], default=default)]) + with pytest.raises(ValueError, match="Mapping keys collide after stringification: '1'"): + walk_click_tree(command) + def test_describes_arguments_and_options(self): @click.group() def root():