Skip to content

Commit 7489f4d

Browse files
committed
Address review comments
1 parent 9e93017 commit 7489f4d

File tree

6 files changed

+14
-134
lines changed

6 files changed

+14
-134
lines changed

langchain-interpreter/src/afm_cli/__init__.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@
108108
from .tools import (
109109
MCPClient,
110110
MCPManager,
111-
build_auth_headers,
112111
filter_tools,
113112
)
114113
from .variables import (
@@ -199,7 +198,6 @@
199198
# MCP Classes
200199
"MCPClient",
201200
"MCPManager",
202-
"build_auth_headers",
203201
"filter_tools",
204202
# Interface Functions
205203
"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

@@ -54,42 +53,6 @@ def auth_flow(self, request: httpx.Request):
5453
yield request
5554

5655

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

langchain-interpreter/tests/test_mcp.py

Lines changed: 2 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
StdioTransport,
2424
ToolFilter,
2525
Tools,
26-
build_auth_headers,
2726
filter_tools,
2827
)
2928
from afm_cli.tools.mcp import (
@@ -114,89 +113,6 @@ class MockArgsSchema(BaseModel):
114113
# =============================================================================
115114

116115

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

@@ -406,7 +322,7 @@ def test_build_connection_config_http_with_auth(self):
406322
config = client._build_connection_config()
407323

408324
assert "auth" in config
409-
assert isinstance(config["auth"], BearerAuth)
325+
assert isinstance(config.get("auth"), BearerAuth)
410326

411327
# ---- Stdio transport tests ----
412328

@@ -489,7 +405,7 @@ def test_build_connection_config_stdio_no_args_defaults_to_empty_list(self):
489405
)
490406
config = client._build_connection_config()
491407

492-
assert config["args"] == []
408+
assert config.get("args") == []
493409

494410
# ---- Shared async tests ----
495411

0 commit comments

Comments
 (0)