Skip to content

Commit ff42321

Browse files
committed
Address review comments
1 parent e2a3603 commit ff42321

File tree

6 files changed

+13
-134
lines changed

6 files changed

+13
-134
lines changed

langchain-interpreter/src/afm_cli/__init__.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@
106106
from .tools import (
107107
MCPClient,
108108
MCPManager,
109-
build_auth_headers,
110109
filter_tools,
111110
)
112111
from .variables import (
@@ -195,7 +194,6 @@
195194
# MCP Classes
196195
"MCPClient",
197196
"MCPManager",
198-
"build_auth_headers",
199197
"filter_tools",
200198
# Interface Functions
201199
"get_interfaces",

langchain-interpreter/src/afm_cli/interfaces/web_chat.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -182,17 +182,22 @@ async def chat_string(
182182

183183
except HTTPException:
184184
raise
185-
except json.JSONDecodeError:
185+
except json.JSONDecodeError as e:
186186
raise HTTPException(
187187
status_code=400,
188188
detail="Invalid JSON in request body",
189-
)
189+
) from e
190+
except UnicodeDecodeError as e:
191+
raise HTTPException(
192+
status_code=400,
193+
detail="Request body must be valid UTF-8",
194+
) from e
190195
except Exception as e:
191196
logger.exception(f"Error in chat_string for session {session_id}: {e}")
192197
raise HTTPException(
193198
status_code=500,
194199
detail="Internal server error",
195-
)
200+
) from e
196201

197202
else:
198203
# Complex schema-based chat
@@ -255,17 +260,17 @@ async def chat_object(
255260

256261
except HTTPException:
257262
raise
258-
except json.JSONDecodeError:
263+
except json.JSONDecodeError as e:
259264
raise HTTPException(
260265
status_code=400,
261266
detail="Invalid JSON in request body",
262-
)
267+
) from e
263268
except Exception as e:
264269
logger.exception(f"Error in chat_object for session {session_id}: {e}")
265270
raise HTTPException(
266271
status_code=500,
267272
detail="Internal server error",
268-
)
273+
) from e
269274

270275
return router
271276

langchain-interpreter/src/afm_cli/models.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class ClientAuthentication(BaseModel):
4141
@model_validator(mode="after")
4242
def validate_type_fields(self) -> Self:
4343
"""Validate that the required fields for each type are present."""
44-
match self.type:
44+
match self.type.lower():
4545
case "bearer":
4646
if self.token is None:
4747
raise ValueError("type 'bearer' requires 'token' field")

langchain-interpreter/src/afm_cli/tools/__init__.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,11 @@
1010
from .mcp import (
1111
MCPClient,
1212
MCPManager,
13-
build_auth_headers,
1413
filter_tools,
1514
)
1615

1716
__all__ = [
1817
"MCPClient",
1918
"MCPManager",
20-
"build_auth_headers",
2119
"filter_tools",
2220
]

langchain-interpreter/src/afm_cli/tools/mcp.py

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
from __future__ import annotations
77

8-
import base64
98
import logging
109
from typing import Any
1110

@@ -52,42 +51,6 @@ def auth_flow(self, request: httpx.Request):
5251
yield request
5352

5453

55-
def build_auth_headers(auth: ClientAuthentication | None) -> dict[str, str]:
56-
"""Build HTTP headers from authentication configuration."""
57-
if auth is None:
58-
return {}
59-
60-
auth_type = auth.type.lower()
61-
62-
if auth_type == "bearer":
63-
if auth.token is None:
64-
raise MCPAuthenticationError("Bearer auth requires 'token' field")
65-
return {"Authorization": f"Bearer {auth.token}"}
66-
67-
elif auth_type == "basic":
68-
if auth.username is None or auth.password is None:
69-
raise MCPAuthenticationError(
70-
"Basic auth requires 'username' and 'password' fields"
71-
)
72-
credentials = base64.b64encode(
73-
f"{auth.username}:{auth.password}".encode()
74-
).decode()
75-
return {"Authorization": f"Basic {credentials}"}
76-
77-
elif auth_type == "api-key":
78-
if auth.api_key is None:
79-
raise MCPAuthenticationError("API key auth requires 'api_key' field")
80-
return {"Authorization": auth.api_key}
81-
82-
elif auth_type in ("oauth2", "jwt"):
83-
raise MCPAuthenticationError(
84-
f"Authentication type '{auth_type}' not yet supported"
85-
)
86-
87-
else:
88-
raise MCPAuthenticationError(f"Unsupported authentication type: {auth_type}")
89-
90-
9154
def build_httpx_auth(auth: ClientAuthentication | None) -> httpx.Auth | None:
9255
"""Build httpx Auth instance from authentication configuration."""
9356
if auth is None:

langchain-interpreter/tests/test_mcp.py

Lines changed: 1 addition & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@
2121
MCPServer,
2222
ToolFilter,
2323
Tools,
24-
Transport,
25-
build_auth_headers,
2624
filter_tools,
2725
)
2826
from afm_cli.tools.mcp import (
@@ -97,89 +95,6 @@ class MockArgsSchema(BaseModel):
9795
# =============================================================================
9896

9997

100-
class TestBuildAuthHeaders:
101-
"""Tests for build_auth_headers function."""
102-
103-
def test_none_auth_returns_empty_dict(self):
104-
"""No auth returns empty headers."""
105-
result = build_auth_headers(None)
106-
assert result == {}
107-
108-
def test_bearer_auth_returns_bearer_header(self):
109-
"""Bearer auth returns Authorization: Bearer header."""
110-
auth = ClientAuthentication(type="bearer", token="my-token")
111-
result = build_auth_headers(auth)
112-
assert result == {"Authorization": "Bearer my-token"}
113-
114-
def test_basic_auth_returns_basic_header(self):
115-
"""Basic auth returns base64-encoded Authorization header."""
116-
auth = ClientAuthentication(type="basic", username="user", password="pass")
117-
result = build_auth_headers(auth)
118-
# "user:pass" -> base64 "dXNlcjpwYXNz"
119-
assert result == {"Authorization": "Basic dXNlcjpwYXNz"}
120-
121-
def test_api_key_auth_returns_api_key_header(self):
122-
"""API key auth returns Authorization header with raw key."""
123-
auth = ClientAuthentication(type="api-key", api_key="my-api-key")
124-
result = build_auth_headers(auth)
125-
assert result == {"Authorization": "my-api-key"}
126-
127-
def test_bearer_auth_missing_token_raises_pydantic_error(self):
128-
"""Bearer auth without token raises Pydantic ValidationError at model creation."""
129-
# Pydantic validates at model creation time, not at build_auth_headers
130-
from pydantic import ValidationError
131-
132-
with pytest.raises(ValidationError, match="requires 'token' field"):
133-
ClientAuthentication(type="bearer")
134-
135-
def test_basic_auth_missing_username_raises_pydantic_error(self):
136-
"""Basic auth without username raises Pydantic ValidationError at model creation."""
137-
from pydantic import ValidationError
138-
139-
with pytest.raises(ValidationError, match="requires 'username' and 'password'"):
140-
ClientAuthentication(type="basic", password="pass")
141-
142-
def test_basic_auth_missing_password_raises_pydantic_error(self):
143-
"""Basic auth without password raises Pydantic ValidationError at model creation."""
144-
from pydantic import ValidationError
145-
146-
with pytest.raises(ValidationError, match="requires 'username' and 'password'"):
147-
ClientAuthentication(type="basic", username="user")
148-
149-
def test_api_key_auth_missing_key_raises_pydantic_error(self):
150-
"""API key auth without key raises Pydantic ValidationError at model creation."""
151-
from pydantic import ValidationError
152-
153-
with pytest.raises(ValidationError, match="requires 'api_key' field"):
154-
ClientAuthentication(type="api-key")
155-
156-
def test_oauth2_auth_not_supported(self):
157-
"""OAuth2 auth raises not supported error."""
158-
auth = ClientAuthentication(type="oauth2")
159-
with pytest.raises(MCPAuthenticationError, match="not yet supported"):
160-
build_auth_headers(auth)
161-
162-
def test_jwt_auth_not_supported(self):
163-
"""JWT auth raises not supported error."""
164-
auth = ClientAuthentication(type="jwt")
165-
with pytest.raises(MCPAuthenticationError, match="not yet supported"):
166-
build_auth_headers(auth)
167-
168-
def test_unknown_auth_type_raises_error(self):
169-
"""Unknown auth type raises MCPAuthenticationError."""
170-
auth = ClientAuthentication(type="unknown")
171-
with pytest.raises(
172-
MCPAuthenticationError, match="Unsupported authentication type"
173-
):
174-
build_auth_headers(auth)
175-
176-
def test_auth_type_is_case_insensitive(self):
177-
"""Auth type matching is case-insensitive."""
178-
auth = ClientAuthentication(type="BEARER", token="my-token")
179-
result = build_auth_headers(auth)
180-
assert result == {"Authorization": "Bearer my-token"}
181-
182-
18398
class TestBuildHttpxAuth:
18499
"""Tests for build_httpx_auth function."""
185100

@@ -385,7 +300,7 @@ def test_build_connection_config_with_auth(self):
385300
config = client._build_connection_config()
386301

387302
assert "auth" in config
388-
assert isinstance(config["auth"], BearerAuth)
303+
assert isinstance(config.get("auth"), BearerAuth)
389304

390305
@pytest.mark.asyncio
391306
async def test_get_tools_calls_mcp_client(self):

0 commit comments

Comments
 (0)