Skip to content

Commit 04f0c8c

Browse files
committed
fix: pre-PR review fixes — token error routing, health check crash, Dockerfile
Three bugs found in code review: 1. Missing token now raises WebexTokenMissingError (common.py) instead of bare RuntimeError. Each tool's _map_exception_to_error checks isinstance first and returns E401 (non-retryable) rather than the catch-all E600 with temporary=True, which was incorrectly suggesting callers should retry a permanent configuration error. 2. health_check.py check_api_connectivity() and check_room_access() called webex_api.people.me() / webex_api.rooms.list() on the module-level value which is now None when WEBEX_ACCESS_TOKEN is unset (HTTP-transport mode). Both functions now return status="skipped" instead of crashing with AttributeError. Docker HEALTHCHECK (--skip-api) was already safe; this fixes direct script invocation. 3. Dockerfile COPY included uv.lock but the builder uses pip which ignores it entirely, giving false confidence in build reproducibility. Removed. https://claude.ai/code/session_01UBF6bZJi742Y4CNg7KdU6N
1 parent c987492 commit 04f0c8c

7 files changed

Lines changed: 30 additions & 8 deletions

File tree

Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ RUN apt-get update && apt-get install -y \
88
WORKDIR /app
99

1010
# Copy everything needed for a PEP 517 build
11-
COPY pyproject.toml uv.lock README.md ./
11+
COPY pyproject.toml README.md ./
1212
COPY src/ ./src/
1313

1414
# Non-editable install so the package lands fully in site-packages

src/webex_bot_mcp/health_check.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@
1818

1919
def check_api_connectivity() -> Dict[str, Any]:
2020
"""Test basic API connectivity and authentication"""
21+
if webex_api is None:
22+
return {
23+
"status": "skipped",
24+
"note": "WEBEX_ACCESS_TOKEN not set; API checks require a token (HTTP transport uses per-request tokens)"
25+
}
2126
try:
2227
me = webex_api.people.me()
2328
return {
@@ -26,7 +31,7 @@ def check_api_connectivity() -> Dict[str, Any]:
2631
"bot_name": me.displayName,
2732
"bot_email": me.emails[0] if me.emails else "unknown",
2833
"org_id": me.orgId,
29-
"response_time_ms": 0 # Would need timing logic
34+
"response_time_ms": 0
3035
}
3136
except Exception as e:
3237
return {
@@ -38,8 +43,13 @@ def check_api_connectivity() -> Dict[str, Any]:
3843

3944
def check_room_access() -> Dict[str, Any]:
4045
"""Check bot's access to rooms"""
46+
if webex_api is None:
47+
return {
48+
"status": "skipped",
49+
"note": "WEBEX_ACCESS_TOKEN not set; room checks require a token"
50+
}
4151
try:
42-
rooms = list(webex_api.rooms.list(max=10)) # Limit to 10 for health check
52+
rooms = list(webex_api.rooms.list(max=10))
4353

4454
room_types = {}
4555
for room in rooms:

src/webex_bot_mcp/tools/common.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ class WebexErrorCodes:
3535
TOKEN_EXPIRED = "E602"
3636

3737

38+
class WebexTokenMissingError(RuntimeError):
39+
"""Raised when no Webex token is available for the current request."""
40+
41+
3842
def create_error_response(
3943
error_code: str,
4044
message: str,
@@ -120,7 +124,7 @@ def get_webex_api() -> WebexAPI:
120124
env_token = os.getenv("WEBEX_ACCESS_TOKEN")
121125
if env_token:
122126
return _cached_webex_api(env_token)
123-
raise RuntimeError(
127+
raise WebexTokenMissingError(
124128
"No Webex token available. Provide an 'Authorization: Bearer <token>' "
125129
"header (HTTP transport) or set WEBEX_ACCESS_TOKEN (stdio transport)."
126130
)

src/webex_bot_mcp/tools/memberships.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22
Webex Membership management tools.
33
"""
44
from typing import Optional, Dict, Any
5-
from .common import get_webex_api, create_error_response, create_success_response, WebexErrorCodes
5+
from .common import get_webex_api, create_error_response, create_success_response, WebexErrorCodes, WebexTokenMissingError
66

77

88
def _map_exception_to_error(e: Exception) -> Dict[str, Any]:
9+
if isinstance(e, WebexTokenMissingError):
10+
return create_error_response(WebexErrorCodes.UNAUTHORIZED, str(e))
911
error_str = str(e).lower()
1012
if 'unauthorized' in error_str or 'invalid token' in error_str:
1113
return create_error_response(WebexErrorCodes.UNAUTHORIZED,

src/webex_bot_mcp/tools/messages.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
Webex Message management tools.
33
"""
44
from typing import Optional, Dict, Any, List
5-
from .common import get_webex_api, create_error_response, create_success_response, WebexErrorCodes
5+
from .common import get_webex_api, create_error_response, create_success_response, WebexErrorCodes, WebexTokenMissingError
66

77

88
def format_mention_by_email(email: str, display_name: Optional[str] = None) -> str:
@@ -67,6 +67,8 @@ def create_message_with_mentions(
6767

6868
def _map_exception_to_error(e: Exception) -> Dict[str, Any]:
6969
"""Map a caught exception to a structured error response."""
70+
if isinstance(e, WebexTokenMissingError):
71+
return create_error_response(WebexErrorCodes.UNAUTHORIZED, str(e))
7072
error_str = str(e).lower()
7173
if 'rate limit' in error_str or 'too many requests' in error_str:
7274
return create_error_response(

src/webex_bot_mcp/tools/people.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22
Webex People management tools.
33
"""
44
from typing import Optional, Dict, Any
5-
from .common import get_webex_api, create_error_response, create_success_response, WebexErrorCodes
5+
from .common import get_webex_api, create_error_response, create_success_response, WebexErrorCodes, WebexTokenMissingError
66

77

88
def _map_exception_to_error(e: Exception) -> Dict[str, Any]:
9+
if isinstance(e, WebexTokenMissingError):
10+
return create_error_response(WebexErrorCodes.UNAUTHORIZED, str(e))
911
error_str = str(e).lower()
1012
if 'unauthorized' in error_str or 'invalid token' in error_str:
1113
return create_error_response(WebexErrorCodes.UNAUTHORIZED,

src/webex_bot_mcp/tools/rooms.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22
Webex Room/Space management tools.
33
"""
44
from typing import Optional, Dict, Any
5-
from .common import get_webex_api, create_error_response, create_success_response, WebexErrorCodes
5+
from .common import get_webex_api, create_error_response, create_success_response, WebexErrorCodes, WebexTokenMissingError
66

77

88
def _map_exception_to_error(e: Exception) -> Dict[str, Any]:
9+
if isinstance(e, WebexTokenMissingError):
10+
return create_error_response(WebexErrorCodes.UNAUTHORIZED, str(e))
911
error_str = str(e).lower()
1012
if 'unauthorized' in error_str or 'invalid token' in error_str:
1113
return create_error_response(WebexErrorCodes.UNAUTHORIZED,

0 commit comments

Comments
 (0)