feat: add Choice provider for system selection (#592)

* feat: add Choice provider for system selection and integration tests

- Add fastmcp[apps] Choice provider so the LLM presents clickable
  system options to the user before calling sap_login
- Server instructions dynamically list available system keys and
  guide the LLM to use the choose tool
- Update README getting-started section: dict keys are arbitrary,
  connection_name field matches SAP Logon entry
- Add unit tests for connection_name resolution (both backends)
- Add MCP protocol integration tests via FastMCP Client

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: recompile requirements.txt with fastmcp[apps] and fix formatting

requirements.txt was missing prefab-ui (needed by fastmcp.apps.choice),
causing CI import failures.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: bump fastmcp minimum to 3.2.0 (fastmcp.apps added in 3.2.0)

fastmcp.apps.choice.Choice doesn't exist in 3.1.1 — the apps
subpackage was introduced in 3.2.0.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: narrow exception catch to satisfy pylint broad-exception-caught

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: rename connection_name→system_key on sap_login, expose configured systems without passwords (#594)

Disambiguate the two meanings of "connection_name":
- sap_login parameter renamed to system_key (the systems.json dict key)
- configured_systems response uses sap_logon_entry (the SAP Logon pad description)

Also: sap_list_connections now returns configured_systems from systems.json
(key, sap_logon_entry, host, client, language — no passwords/users).
Unknown system_key returns an error instead of silently falling back to default.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: hf-kklein

README.md

@@ -93,20 +93,28 @@ On **Windows**, open Windows Explorer and paste this into the address bar:
 
 Create the folder if it doesn't exist, then create a file called `systems.json` inside it. On **macOS/Linux**, the path is `~/.config/sap-mcp/systems.json`.
 
-**System key = SAP Logon description, NOT the SID.** The key in `"systems"` (e.g. `"HF S/4"`) must match the **description** shown in SAP Logon — the bold text in the connection list. It is _not_ the 3-character System ID (SID) like `HFQ` or `DEV`.
+There are two distinct identifiers per system — don't mix them up:
 
-| What you see in SAP Logon | Use as system key         | NOT this (SID) |
-| ------------------------- | ------------------------- | -------------- |
-| **HF S/4**                | `"HF S/4"`                | ~~`"HFQ"`~~    |
-| **DEV - ERP Development** | `"DEV - ERP Development"` | ~~`"DEV"`~~    |
+| Concept | Example | Where it's used |
+| --- | --- | --- |
+| **System key** (dictionary key) | `"dev"`, `"qa"` | `sap_login(system_key="qa")` — selects which system to log into |
+| **SAP Logon entry** (`connection_name` field) | `"HF S/4"`, `"DEV - ERP Development"` | Must match the **bold description** in the SAP Logon pad exactly |
 
-If the key doesn't match exactly, you'll get _"SAP Logon connection entry not found"_.
+The SAP Logon entry is _not_ the 3-character System ID (SID):
+
+| What you see in SAP Logon | `connection_name` value     | NOT this (SID) |
+| ------------------------- | --------------------------- | -------------- |
+| **HF S/4**                | `"HF S/4"`                  | ~~`"HFQ"`~~    |
+| **DEV - ERP Development** | `"DEV - ERP Development"`   | ~~`"DEV"`~~    |
+
+If the `connection_name` doesn't match exactly, you'll get _"SAP Logon connection entry not found"_.
 
 ```json
 {
-    "default_system": "HF S/4",
+    "default_system": "dev",
     "systems": {
-        "HF S/4": {
+        "dev": {
+            "connection_name": "HF S/4",
             "host": "https://your-sap-system:44300",
             "client": "100",
             "user": "your_username",
@@ -157,10 +165,10 @@ Multi-system support is built into `systems.json` — add multiple systems and t
 
 **How it works:**
 
-1. `sap_list_connections` reads your SAP Logon entries (from `SAPUILandscape.xml`) to show which systems are available.
-2. `sap_login(connection_name="qa")` logs into a specific system using credentials from `systems.json`.
+1. `sap_list_connections` returns both configured systems (from `systems.json`) and SAP Logon entries (from `SAPUILandscape.xml`).
+2. `sap_login(system_key="qa")` logs into a specific system using credentials from `systems.json`.
 
-**Configuration:** Add multiple systems to your `systems.json`. The dictionary key (e.g. `"dev"`, `"qa"`) is used for config lookup. The `connection_name` field must match the SAP Logon entry description:
+**Configuration:** Add multiple systems to your `systems.json`. The **dictionary key** (e.g. `"dev"`, `"qa"`) is the `system_key` you pass to `sap_login`. The `connection_name` field must match the SAP Logon entry description exactly:
 
 ```json
 {
@@ -186,7 +194,7 @@ Multi-system support is built into `systems.json` — add multiple systems and t
 }
 ```
 
-When `sap_login(connection_name="qa")` is called, it looks up the system in `systems.json` and uses the `connection_name` field to open the SAP Logon entry. If not found, it falls back to `default_system`.
+When `sap_login(system_key="qa")` is called, it looks up `"qa"` in `systems.json`, reads the credentials, and uses the `connection_name` field (`"QA System"`) to open the matching SAP Logon entry. If the system key is not found, an error is returned listing the available keys.
 
 No Chrome, no browser setup required.
 
@@ -732,7 +740,7 @@ docker logs sap-mcp-cdp-proxy-1
 
 - Check `SAP_URL` is correct and accessible from your browser
 - If using auto-login, verify credentials are configured in `systems.json` (see [Configuration Reference](#configuration-reference))
-- **Desktop backend:** Make sure the `connection_name` field in `systems.json` matches the SAP Logon **description** exactly (not the SID). Open SAP Logon and compare.
+- **Desktop backend:** Make sure the `connection_name` field in `systems.json` matches the SAP Logon pad **description** exactly (the bold text, not the SID). Open SAP Logon and compare.
 - Try logging in manually first to verify credentials
 
 ### Transaction input field (OK-Code field) not visible

pyproject.toml

@@ -19,7 +19,7 @@ classifiers = [
     "Programming Language :: Python :: 3.14",
 ]
 dependencies = [
-    "fastmcp>=2.14.0,<4.0.0",  # Standalone FastMCP v2.14+, replaces mcp[cli]
+    "fastmcp[apps]>=3.2.0,<4.0.0",  # FastMCP v3.2+ required for Choice provider (fastmcp.apps)
     "httpx>=0.27.0",  # HTTP client for GitHub API (feedback issue handler)
     "playwright>=1.40.0",
     "pydantic>=2.0.0",

requirements.txt

@@ -43,7 +43,9 @@ cryptography==46.0.6
     #   pyjwt
     #   secretstorage
 cyclopts==4.8.0
-    # via fastmcp
+    # via
+    #   fastmcp
+    #   prefab-ui
 dnspython==2.8.0
     # via email-validator
 docstring-parser==0.17.0
@@ -54,7 +56,7 @@ email-validator==2.3.0
     # via pydantic
 exceptiongroup==1.3.1
     # via fastmcp
-fastmcp==3.1.1
+fastmcp[apps]==3.2.0
     # via sapwebguimcp (pyproject.toml)
 greenlet==3.3.2
     # via playwright
@@ -120,6 +122,8 @@ platformdirs==4.9.4
     # via fastmcp
 playwright==1.58.0
     # via sapwebguimcp (pyproject.toml)
+prefab-ui==0.18.1
+    # via fastmcp
 py-key-value-aio[filetree,keyring,memory]==0.4.4
     # via fastmcp
 pycparser==3.0
@@ -129,6 +133,7 @@ pydantic[email]==2.12.5
     #   fastmcp
     #   mcp
     #   openapi-pydantic
+    #   prefab-ui
     #   pydantic-settings
     #   sap-mcp-config
     #   sapsucker
@@ -170,6 +175,7 @@ rich==14.3.3
     # via
     #   cyclopts
     #   fastmcp
+    #   prefab-ui
     #   rich-rst
 rich-rst==1.3.2
     # via cyclopts

src/sapwebguimcp/models/config.py

@@ -253,13 +253,14 @@ def credentials_for(self, system_key: str) -> tuple[str, str]:
         """Return (user, password) for a system key.
 
         Looks up ``system_key`` in the shared SAP config
-        (``~/.config/sap-mcp/systems.json``).  Falls back to the default
-        system when the key is not found.
+        (``~/.config/sap-mcp/systems.json``).  Raises ``KeyError`` when the
+        key is not found.
         """
         sap_cfg = get_sap_config()
         system = sap_cfg.systems.get(system_key)
         if system is None:
-            system = sap_cfg.get_default()
+            available = list(sap_cfg.systems.keys())
+            raise KeyError(f"System key {system_key!r} not found in systems.json. " f"Available: {available}")
         return system.user, system.password.get_secret_value()
 
     def validate_for_browser(self) -> list[str]:

src/sapwebguimcp/server.py

@@ -11,12 +11,13 @@
 
 import httpx
 from fastmcp import FastMCP
+from fastmcp.apps.choice import Choice
 from fastmcp.server.middleware.logging import LoggingMiddleware
 
 from sapwebguimcp.backend.manager import close_backend
 from sapwebguimcp.logging_config import configure_logging
 from sapwebguimcp.middleware import ToolCallLoggingMiddleware
-from sapwebguimcp.models.config import get_settings
+from sapwebguimcp.models.config import get_sap_config, get_settings
 from sapwebguimcp.prompts import register_prompts
 from sapwebguimcp.resources import register_feedback_resources, register_intent_resources
 from sapwebguimcp.tools import (
@@ -178,14 +179,14 @@ async def app_lifespan(_server: FastMCP) -> AsyncIterator[None]:
 
 CONNECTION:
 The server connects using the default_system from ~/.config/sap-mcp/systems.json.
-Each system entry has a connection_name field that identifies the SAP Logon entry.
-Override via the connection_name parameter in sap_login() (pass the system key from the config).
+Each system entry has a sap_logon_entry field (the bold description text in the SAP Logon pad).
+Override via the system_key parameter in sap_login() (pass the dictionary key from systems.json).
 
 IF CONNECTION FAILS:
 Ask the user to verify:
 1. "Is SAP GUI running?" (SAP Logon must be open)
 2. "Is scripting enabled?" (both server-side RZ11 and client-side SAP GUI Options)
-3. "Does the connection_name in systems.json match the SAP Logon entry description?"
+3. "Does the sap_logon_entry in systems.json match the SAP Logon pad description exactly?"
 
 COMMON ERROR CAUSES:
 - RPC_E_DISCONNECTED: SAP GUI closed or session timed out — call sap_login() again
@@ -209,7 +210,27 @@ async def app_lifespan(_server: FastMCP) -> AsyncIterator[None]:
   - VKey codes: 0=Enter, 3=F3/Back, 8=F8/Execute, 11=F11/Save, 12=F12/Cancel
 """
 
-SERVER_INSTRUCTIONS = _DESKTOP_INSTRUCTIONS if _settings.backend_type == "desktop" else _WEBGUI_INSTRUCTIONS
+
+def _build_instructions() -> str:
+    base = _DESKTOP_INSTRUCTIONS if _settings.backend_type == "desktop" else _WEBGUI_INSTRUCTIONS
+    try:
+        sap_cfg = get_sap_config()
+        keys = list(sap_cfg.systems.keys())
+        default = sap_cfg.default_system
+        systems_info = (
+            f"\nAVAILABLE SYSTEMS (from systems.json):\n"
+            f"Default: {default!r}\n"
+            f"All system keys: {keys}\n"
+            f"When multiple systems are configured, use the choose tool to let the user "
+            f"pick a system before calling sap_login(). "
+            f"Pass the selected key as system_key to sap_login().\n"
+        )
+        return base + systems_info
+    except (FileNotFoundError, ValueError):  # config not found or invalid — don't crash
+        return base
+
+
+SERVER_INSTRUCTIONS = _build_instructions()
 
 # Create the FastMCP server instance with strict input validation
 _SERVER_NAME = "sap-desktop-mcp" if _settings.backend_type == "desktop" else "sap-webgui-mcp"
@@ -223,6 +244,9 @@ async def app_lifespan(_server: FastMCP) -> AsyncIterator[None]:
 # Add logging middleware for tool call sequence analysis
 mcp.add_middleware(ToolCallLoggingMiddleware())
 
+# Add Choice provider so the LLM can present system selection UI to the user
+mcp.add_provider(Choice())
+
 # Add FastMCP built-in logging with payload visibility
 mcp.add_middleware(LoggingMiddleware(include_payloads=True, max_payload_length=1000))
 

src/sapwebguimcp/tools/sap_list_connections_impl.py

@@ -7,6 +7,7 @@
 from sapwebguimcp.backend.desktop._landscape import _find_landscape_path, _parse_landscape_xml
 from sapwebguimcp.backend.manager import get_backend
 from sapwebguimcp.models.base import ToolResult
+from sapwebguimcp.models.config import get_sap_config
 
 __all__ = ["sap_list_connections_impl", "_parse_landscape_xml"]
 
@@ -15,18 +16,52 @@ class ConnectionListResult(ToolResult):
     """Result from sap_list_connections tool."""
 
     connections: list[dict[str, Any]] = Field(default_factory=list)
+    configured_systems: list[dict[str, str]] = Field(default_factory=list)
+
+
+def _get_configured_systems() -> list[dict[str, str]]:
+    """Return systems from systems.json without sensitive fields."""
+    try:
+        sap_cfg = get_sap_config()
+    except (FileNotFoundError, ValueError):
+        return []
+    result = []
+    for key, system in sap_cfg.systems.items():
+        result.append(
+            {
+                "key": key,
+                "sap_logon_entry": system.connection_name,
+                "host": system.host,
+                "client": system.client,
+                "language": system.language,
+            }
+        )
+    return result
 
 
 async def sap_list_connections_impl() -> ConnectionListResult:
     """List available SAP Logon connections from the landscape file or backend."""
+    configured_systems = _get_configured_systems()
     try:
         backend = await get_backend(tool_name="sap_list_connections")
         connections = await backend.list_connections()
-        return ConnectionListResult(success=True, connections=connections)
-    except Exception as e:  # pylint: disable=broad-exception-caught
+        return ConnectionListResult(
+            success=True,
+            connections=connections,
+            configured_systems=configured_systems,
+        )
+    except Exception:  # pylint: disable=broad-exception-caught
         # Fall back to reading the landscape file directly
         path = _find_landscape_path()
         if path is None:
-            return ConnectionListResult.failure(f"Could not find SAPUILandscape.xml: {e}")
+            return ConnectionListResult(
+                success=True,
+                connections=[],
+                configured_systems=configured_systems,
+            )
         connections = _parse_landscape_xml(path.read_text(encoding="utf-8"))
-        return ConnectionListResult(success=True, connections=connections)
+        return ConnectionListResult(
+            success=True,
+            connections=connections,
+            configured_systems=configured_systems,
+        )

src/sapwebguimcp/tools/sap_login_impl.py

@@ -12,7 +12,7 @@
 async def sap_login_impl(
     url: Optional[str] = None,
     client: Optional[str] = None,
-    connection_name: Optional[str] = None,
+    system_key: Optional[str] = None,
     session_id: Optional[str] = None,
 ) -> LoginResult:
     """
@@ -21,7 +21,7 @@ async def sap_login_impl(
     Args:
         url: SAP Web GUI URL (WebGUI only). If not provided, derives from shared config or SAP_URL.
         client: SAP client/mandant (3-digit string). If not provided, uses shared config.
-        connection_name: System key from systems.json. Overrides default_system.
+        system_key: Dictionary key from systems.json (e.g. "dev-100"). Overrides default_system.
         session_id: Session ID for multi-session support.
 
     Returns:
@@ -30,14 +30,22 @@ async def sap_login_impl(
     settings = get_settings()
     sap_cfg = get_sap_config()
 
-    effective_key = connection_name or sap_cfg.default_system
-    system = sap_cfg.systems.get(effective_key) or sap_cfg.get_default()
+    effective_key = system_key or sap_cfg.default_system
+    system = sap_cfg.systems.get(effective_key)
+    if system is None:
+        if system_key is not None:
+            return LoginResult.failure(
+                f"System key {system_key!r} not found in systems.json. "
+                f"Available system keys: {list(sap_cfg.systems.keys())}"
+            )
+        system = sap_cfg.get_default()
+
     user, password = system.user, system.password.get_secret_value()
     effective_client = client or system.client
     effective_url = url or settings.sap_url or (system.host + "/sap/bc/gui/sap/its/webgui")
     language = system.language
 
-    # URL check only applies to WebGUI -- Desktop uses connection_name instead
+    # URL check only applies to WebGUI -- Desktop uses the SAP Logon entry instead
     if settings.backend_type == "webgui" and not effective_url:
         return LoginResult.failure(
             "No SAP URL provided. Either pass a URL parameter or set SAP_URL, "

src/sapwebguimcp/tools/sap_tools.py

@@ -244,13 +244,14 @@ async def sap_keepalive_stop() -> KeepaliveResult:
             "On WebGUI: requires Chrome with --remote-debugging-port=9222 and VPN (if internal SAP). "
             "On Desktop: requires SAP GUI for Windows with scripting enabled. "
             "Credentials are read from ~/.config/sap-mcp/systems.json. "
-            "Use client or connection_name (system key from systems.json) to override the default system."
+            "Use system_key (dictionary key from systems.json) to select a non-default system. "
+            "Call sap_list_connections first to see available system keys."
         )
     )
     async def sap_login(
         url: Optional[str] = None,
         client: Optional[str] = None,
-        connection_name: Optional[str] = None,
+        system_key: Optional[str] = None,
         ctx: Context | None = None,
     ) -> LoginResult:
         """
@@ -263,24 +264,27 @@ async def sap_login(
         Args:
             url: SAP Web GUI URL (WebGUI only). If not provided, derived from system host.
             client: SAP client/mandant (3-digit string, e.g. '200'). Overrides config value.
-            connection_name: System key from systems.json. The system's connection_name
-                field identifies the SAP Logon entry.
+            system_key: Dictionary key from systems.json (e.g. "dev", "qa").
+                Use sap_list_connections to see available keys. Defaults to default_system.
 
         Returns:
             LoginResult indicating login success or what action is needed.
         """
         session_id = getattr(ctx, "session_id", None) if ctx else None
-        return await sap_login_impl(url=url, client=client, connection_name=connection_name, session_id=session_id)
+        return await sap_login_impl(url=url, client=client, system_key=system_key, session_id=session_id)
 
     @mcp.tool(
         description=(
-            "List available SAP systems from SAP Logon (SAPUILandscape.xml). "
-            "Returns connection names that can be passed to sap_login as connection_name. "
-            "Desktop backend only."
+            "List available SAP systems. Returns two sections: "
+            "'configured_systems' from systems.json (pass the 'key' field as "
+            "system_key to sap_login), and 'connections' from SAP Logon "
+            "(SAPUILandscape.xml, Desktop only). "
+            "IMPORTANT: pass a configured_systems 'key' as system_key to sap_login, "
+            "NOT a SAP Logon entry name."
         )
     )
     async def sap_list_connections() -> ConnectionListResult:
-        """List available SAP Logon connections."""
+        """List configured systems and SAP Logon entries."""
         return await sap_list_connections_impl()
 
     @mcp.tool(