Phase 6: Model Validation & Dead Code Cleanup

src/keboola_agent_cli/client.py

@@ -105,7 +105,7 @@ def verify_token(self) -> TokenVerifyResponse:
         return TokenVerifyResponse(
             token_id=str(data.get("id", "")),
             token_description=data.get("description", ""),
-            project_id=data.get("owner", {}).get("id", 0),
+            project_id=data.get("owner", {}).get("id"),
             project_name=data.get("owner", {}).get("name", ""),
             owner_name=data.get("owner", {}).get("name", ""),
         )

src/keboola_agent_cli/models.py

@@ -13,7 +13,7 @@ class ProjectConfig(BaseModel):
     project_name: str = Field(
         default="", description="Human-readable project name (populated on add)"
     )
-    project_id: int = Field(default=0, description="Keboola project ID (populated on add)")
+    project_id: int | None = Field(default=None, description="Keboola project ID (populated on add)")
 
     @field_validator("stack_url")
     @classmethod
@@ -46,11 +46,11 @@ class AppConfig(BaseModel):
 class TokenVerifyResponse(BaseModel):
     """Response from the Keboola token verification endpoint."""
 
-    token_id: str = Field(default="", description="Token identifier")
-    token_description: str = Field(default="", description="Human-readable token description")
-    project_id: int = Field(default=0, description="Keboola project numeric ID")
-    project_name: str = Field(default="", description="Keboola project name")
-    owner_name: str = Field(default="", description="Project owner name")
+    token_id: str = Field(description="Token identifier")
+    token_description: str = Field(description="Human-readable token description")
+    project_id: int | None = Field(default=None, description="Keboola project numeric ID")
+    project_name: str = Field(description="Keboola project name")
+    owner_name: str = Field(description="Project owner name")
 
 
 class ErrorResponse(BaseModel):

src/keboola_agent_cli/output.py

@@ -3,6 +3,7 @@
 import json
 import sys
 from collections.abc import Callable
+from datetime import datetime
 from typing import Any
 
 from rich.console import Console
@@ -192,9 +193,7 @@ def format_config_detail(console: Console, data: dict[str, Any]) -> None:
     # Show configuration parameters if present
     configuration = data.get("configuration", {})
     if configuration:
-        import json as _json
-
-        config_str = _json.dumps(configuration, indent=2)
+        config_str = json.dumps(configuration, indent=2)
         lines.append(f"\n[bold]Configuration:[/bold]\n{config_str}")
 
     # Show rows if present
@@ -298,8 +297,6 @@ def _format_duration(job: dict[str, Any]) -> str:
     start = job.get("startTime")
     end = job.get("endTime")
     if start and end:
-        from datetime import datetime
-
         try:
             start_dt = datetime.fromisoformat(start)
             end_dt = datetime.fromisoformat(end)
@@ -699,6 +696,4 @@ def format_doctor_panel(console: Console, data: dict[str, Any]) -> None:
     lines.append("")
     lines.append(f"  Summary: {', '.join(parts)}")
 
-    from rich.panel import Panel
-
     console.print(Panel("\n".join(lines), title="kbagent doctor", expand=False))

src/keboola_agent_cli/services/mcp_service.py

@@ -302,9 +302,10 @@ async def _connect_and_auto_expand(
 
 
 def _extract_ids(content_items: list[Any], key: str) -> list[str]:
-    """Extract ID values from parsed MCP tool content.
+    """Extract unique ID values from parsed MCP tool content.
 
     Handles both list-of-dicts format and single-dict-with-list format.
+    Deduplicates while preserving insertion order.
     """
     ids = []
     for item in content_items:
@@ -321,7 +322,7 @@ def _extract_ids(content_items: list[Any], key: str) -> list[str]:
                     for sub in value:
                         if isinstance(sub, dict) and key in sub:
                             ids.append(str(sub[key]))
-    return ids
+    return list(dict.fromkeys(ids))
 
 
 class McpService(BaseService):
@@ -334,13 +335,6 @@ class McpService(BaseService):
     Uses the same DI pattern as JobService/ConfigService.
     """
 
-    def __init__(
-        self,
-        config_store: ConfigStore,
-        client_factory: ClientFactory | None = None,
-    ) -> None:
-        super().__init__(config_store, client_factory)
-
     def resolve_project(self, alias: str | None = None) -> tuple[str, ProjectConfig]:
         """Resolve a single project alias (or the default project).
 
@@ -456,7 +450,7 @@ def validate_tool_input(
         """
         schema = self.get_tool_schema(tool_name, aliases=aliases)
         if schema is None:
-            return []  # Can't validate - tool not found, let it fail at call time
+            return []  # Tool not found; call_tool will raise ConfigError
 
         required = schema.get("required", [])
         missing = [param for param in required if param not in tool_input]
@@ -490,10 +484,22 @@ def call_tool(
 
         Returns:
             Dict with "results" list and "errors" list.
+
+        Raises:
+            ConfigError: If tool_name is not found in the available tool list.
         """
         if tool_input is None:
             tool_input = {}
 
+        # Validate tool name exists in the MCP tool list
+        tool_list_result = self.list_tools(aliases=[alias] if alias else None)
+        known_tools = {t["name"] for t in tool_list_result.get("tools", [])}
+        if known_tools and tool_name not in known_tools:
+            raise ConfigError(
+                f"Unknown MCP tool '{tool_name}'. "
+                f"Use 'kbagent tool list' to see available tools."
+            )
+
         is_write = _is_write_tool(tool_name)
 
         if is_write:

tests/test_client.py

@@ -579,7 +579,7 @@ def test_verify_token_minimal_response(self, httpx_mock) -> None:
             result = client.verify_token()
             assert result.token_id == "1"
             assert result.project_name == ""
-            assert result.project_id == 0
+            assert result.project_id is None
 
 
 class TestLargeResponse:

tests/test_mcp_service.py

@@ -10,6 +10,7 @@
 from keboola_agent_cli.models import ProjectConfig
 from keboola_agent_cli.services.mcp_service import (
     McpService,
+    _extract_ids,
     _is_write_tool,
     detect_mcp_server_command,
 )
@@ -741,3 +742,105 @@ def test_mcp_timeout_defaults(self) -> None:
                 assert mcp_mod.MCP_INIT_TIMEOUT_SECONDS == DEFAULT_MCP_INIT_TIMEOUT
         finally:
             importlib.reload(mcp_mod)
+
+
+# ---------------------------------------------------------------------------
+# TestUnknownToolName
+# ---------------------------------------------------------------------------
+
+
+class TestUnknownToolName:
+    """Tests for Phase 6: Unknown MCP tool name validation."""
+
+    @patch("keboola_agent_cli.services.mcp_service.asyncio.run")
+    def test_unknown_tool_name_error(
+        self, mock_run: MagicMock, tmp_path: Path
+    ) -> None:
+        """Calling a nonexistent tool raises ConfigError with a clear message."""
+        mock_run.return_value = _sample_tools()
+
+        store = _setup_store(
+            tmp_path,
+            projects={"prod": {"token": "tok-prod"}},
+        )
+        svc = McpService(config_store=store)
+
+        with pytest.raises(ConfigError, match="Unknown MCP tool 'nonexistent_tool'"):
+            svc.call_tool("nonexistent_tool", {})
+
+    @patch("keboola_agent_cli.services.mcp_service.asyncio.run")
+    def test_known_tool_name_passes_validation(
+        self, mock_run: MagicMock, tmp_path: Path
+    ) -> None:
+        """Calling a known tool does not raise ConfigError for tool name."""
+        call_count = 0
+        tools = _sample_tools()
+
+        def side_effect(coro):
+            nonlocal call_count
+            call_count += 1
+            if call_count == 1:
+                # First call is list_tools
+                return tools
+            # Second call is the actual tool call
+            return {"content": [{"data": "ok"}], "isError": False}
+
+        mock_run.side_effect = side_effect
+
+        store = _setup_store(
+            tmp_path,
+            projects={"prod": {"token": "tok-prod"}},
+            default_project="prod",
+        )
+        svc = McpService(config_store=store)
+        # create_config is in the sample tools and is a write tool
+        result = svc.call_tool("create_config", {"name": "test"})
+        assert result["results"] or result["errors"]  # No ConfigError raised
+
+
+# ---------------------------------------------------------------------------
+# TestExtractIdsDeduplication
+# ---------------------------------------------------------------------------
+
+
+class TestExtractIdsDeduplication:
+    """Tests for Phase 6: _extract_ids deduplication."""
+
+    def test_extract_ids_deduplication(self) -> None:
+        """_extract_ids removes duplicate IDs while preserving order."""
+        content = [
+            [
+                {"id": "bucket-1"},
+                {"id": "bucket-2"},
+                {"id": "bucket-1"},  # duplicate
+                {"id": "bucket-3"},
+                {"id": "bucket-2"},  # duplicate
+            ]
+        ]
+        result = _extract_ids(content, "id")
+        assert result == ["bucket-1", "bucket-2", "bucket-3"]
+
+    def test_extract_ids_no_duplicates(self) -> None:
+        """_extract_ids works normally when there are no duplicates."""
+        content = [
+            [
+                {"id": "a"},
+                {"id": "b"},
+                {"id": "c"},
+            ]
+        ]
+        result = _extract_ids(content, "id")
+        assert result == ["a", "b", "c"]
+
+    def test_extract_ids_empty_input(self) -> None:
+        """_extract_ids returns empty list for empty input."""
+        result = _extract_ids([], "id")
+        assert result == []
+
+    def test_extract_ids_nested_dict_deduplication(self) -> None:
+        """_extract_ids deduplicates across nested dict formats."""
+        content = [
+            {"id": "x", "items": [{"id": "y"}, {"id": "x"}]},
+        ]
+        result = _extract_ids(content, "id")
+        assert result == ["x", "y"]

tests/test_models.py

@@ -5,7 +5,13 @@
 import pytest
 from pydantic import ValidationError
 
-from keboola_agent_cli.models import AppConfig, ErrorResponse, ProjectConfig, SuccessResponse
+from keboola_agent_cli.models import (
+    AppConfig,
+    ErrorResponse,
+    ProjectConfig,
+    SuccessResponse,
+    TokenVerifyResponse,
+)
 
 
 class TestProjectConfig:
@@ -31,7 +37,7 @@ def test_default_values(self) -> None:
             token="901-token",
         )
         assert config.project_name == ""
-        assert config.project_id == 0
+        assert config.project_id is None
 
     def test_json_round_trip(self) -> None:
         """ProjectConfig can be serialized to JSON and deserialized back."""
@@ -255,6 +261,82 @@ def test_project_add_accepts_https_gcp(self) -> None:
         assert config.stack_url == "https://connection.europe-west3.gcp.keboola.com"
 
 
+class TestTokenVerifyResponseValidation:
+    """Tests for Phase 6: TokenVerifyResponse required fields and project_id default."""
+
+    def test_token_verify_response_rejects_missing_fields(self) -> None:
+        """TokenVerifyResponse with missing required fields raises ValidationError."""
+        with pytest.raises(ValidationError):
+            TokenVerifyResponse(
+                token_id="123",
+                token_description="My Token",
+                # project_name missing
+                # owner_name missing
+            )
+
+    def test_token_verify_response_rejects_missing_owner_name(self) -> None:
+        """TokenVerifyResponse with missing owner_name raises ValidationError."""
+        with pytest.raises(ValidationError, match="owner_name"):
+            TokenVerifyResponse(
+                token_id="123",
+                token_description="My Token",
+                project_name="Test Project",
+                # owner_name missing
+            )
+
+    def test_token_verify_response_rejects_missing_token_id(self) -> None:
+        """TokenVerifyResponse with missing token_id raises ValidationError."""
+        with pytest.raises(ValidationError, match="token_id"):
+            TokenVerifyResponse(
+                token_description="My Token",
+                project_name="Test Project",
+                owner_name="Test Owner",
+            )
+
+    def test_token_verify_response_rejects_missing_token_description(self) -> None:
+        """TokenVerifyResponse with missing token_description raises ValidationError."""
+        with pytest.raises(ValidationError, match="token_description"):
+            TokenVerifyResponse(
+                token_id="123",
+                project_name="Test Project",
+                owner_name="Test Owner",
+            )
+
+    def test_token_verify_response_rejects_missing_project_name(self) -> None:
+        """TokenVerifyResponse with missing project_name raises ValidationError."""
+        with pytest.raises(ValidationError, match="project_name"):
+            TokenVerifyResponse(
+                token_id="123",
+                token_description="My Token",
+                owner_name="Test Owner",
+            )
+
+    def test_project_id_default_none(self) -> None:
+        """TokenVerifyResponse project_id defaults to None, not 0."""
+        response = TokenVerifyResponse(
+            token_id="123",
+            token_description="My Token",
+            project_name="Test Project",
+            owner_name="Test Owner",
+        )
+        assert response.project_id is None
+
+    def test_token_verify_response_with_all_fields(self) -> None:
+        """TokenVerifyResponse with all fields specified works correctly."""
+        response = TokenVerifyResponse(
+            token_id="123",
+            token_description="My Token",
+            project_id=4567,
+            project_name="Test Project",
+            owner_name="Test Owner",
+        )
+        assert response.token_id == "123"
+        assert response.token_description == "My Token"
+        assert response.project_id == 4567
+        assert response.project_name == "Test Project"
+        assert response.owner_name == "Test Owner"
+
+
 class TestMaxParallelWorkersValidation:
     """Tests for max_parallel_workers upper bound validation."""