Skip to content

Fix bugs, standardize error handling, add delete tools, and add tests#1

Open
adamweeks wants to merge 1 commit into
mainfrom
claude/review-mcp-server-ViZ0M
Open

Fix bugs, standardize error handling, add delete tools, and add tests#1
adamweeks wants to merge 1 commit into
mainfrom
claude/review-mcp-server-ViZ0M

Conversation

@adamweeks
Copy link
Copy Markdown
Contributor

Critical bug fixes:

  • Remove ~50 lines of dead/unreachable code after the except block in
    send_webex_message that was left from a half-finished refactor
  • Fix the files parameter in send_webex_message — the SDK requires a list
    but a bare string was being passed, silently breaking file attachments
  • Fix timestamp field in create_error_response/create_success_response;
    REQUEST_TIMESTAMP env var is never set so the field was always empty string;
    now uses datetime.now(timezone.utc).isoformat()

Architecture:

  • Remove double load_dotenv() call (was in both main.py and tools/common.py)
  • Remove redundant WEBEX_ACCESS_TOKEN check in main.py (tools/common.py
    already validates and raises on import)

Error handling — all tools now use structured responses:

  • Standardize rooms.py, memberships.py, people.py, and the remaining
    message functions to use create_error_response / create_success_response
    with WebexErrorCodes (previously returned bare dicts with no error codes)
  • Extract _map_exception_to_error() and _*_to_dict() helpers in each module
    to eliminate repetitive per-field exception handling

New tools (missing CRUD operations):

  • delete_webex_room / delete_webex_space
  • delete_webex_membership
  • delete_webex_message

Code quality:

  • Consolidate duplicate mention-building logic in send_webex_message_with_mentions
    to use the existing create_message_with_mentions helper
  • Rename id parameter to person_id in list_webex_people (shadowed built-in)
  • Update _build_message_params shared helper used by both message send functions

Config / metadata fixes:

  • Fix log level validation: accept "WARNING" not "WARN" (standard Python logging)
  • Fix pyproject.toml: rename dotenv -> python-dotenv, bump version to 1.0.0,
    add real description
  • Fix resources_count in version resource (was 8, now 10)
  • Update tools_count to 31 (added 3 delete tools + 2 delete aliases)

Tests (new):

  • tests/test_messages.py: 38 unit tests covering mention formatters,
    create_message_with_mentions, send_webex_message validation, files wrapping,
    list/delete, response builders, and WebexConfig; mocks webexpythonsdk at
    sys.modules level so no real credentials are needed

Documentation:

  • Add CLAUDE.md with full developer guide: layout, conventions, error code
    ranges, response shape, how to add a new tool, env var reference

https://claude.ai/code/session_01KTZNsi11DycdLENcbUUbgU

Critical bug fixes:
- Remove ~50 lines of dead/unreachable code after the except block in
  send_webex_message that was left from a half-finished refactor
- Fix the files parameter in send_webex_message — the SDK requires a list
  but a bare string was being passed, silently breaking file attachments
- Fix timestamp field in create_error_response/create_success_response;
  REQUEST_TIMESTAMP env var is never set so the field was always empty string;
  now uses datetime.now(timezone.utc).isoformat()

Architecture:
- Remove double load_dotenv() call (was in both main.py and tools/common.py)
- Remove redundant WEBEX_ACCESS_TOKEN check in main.py (tools/common.py
  already validates and raises on import)

Error handling — all tools now use structured responses:
- Standardize rooms.py, memberships.py, people.py, and the remaining
  message functions to use create_error_response / create_success_response
  with WebexErrorCodes (previously returned bare dicts with no error codes)
- Extract _map_exception_to_error() and _*_to_dict() helpers in each module
  to eliminate repetitive per-field exception handling

New tools (missing CRUD operations):
- delete_webex_room / delete_webex_space
- delete_webex_membership
- delete_webex_message

Code quality:
- Consolidate duplicate mention-building logic in send_webex_message_with_mentions
  to use the existing create_message_with_mentions helper
- Rename id parameter to person_id in list_webex_people (shadowed built-in)
- Update _build_message_params shared helper used by both message send functions

Config / metadata fixes:
- Fix log level validation: accept "WARNING" not "WARN" (standard Python logging)
- Fix pyproject.toml: rename dotenv -> python-dotenv, bump version to 1.0.0,
  add real description
- Fix resources_count in version resource (was 8, now 10)
- Update tools_count to 31 (added 3 delete tools + 2 delete aliases)

Tests (new):
- tests/test_messages.py: 38 unit tests covering mention formatters,
  create_message_with_mentions, send_webex_message validation, files wrapping,
  list/delete, response builders, and WebexConfig; mocks webexpythonsdk at
  sys.modules level so no real credentials are needed

Documentation:
- Add CLAUDE.md with full developer guide: layout, conventions, error code
  ranges, response shape, how to add a new tool, env var reference

https://claude.ai/code/session_01KTZNsi11DycdLENcbUUbgU
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants