Skip to content

Add mypy typechecking and improve error handling - #247

Merged
pontino merged 7 commits into
mainfrom
chore/adopt-mypy-and-minor-fixes
Sep 7, 2026
Merged

pontino merged 7 commits into
mainfrom
chore/adopt-mypy-and-minor-fixes

Conversation

@rocky-jaiswal

Copy link
Copy Markdown
Contributor

Summary

Adopts static type checking (mypy) for the eggai package as a CI gate, plus the annotation cleanups required to make it pass. Along the way this fixes a couple of error paths.

@github-actions

github-actions Bot commented Jun 18, 2026 •

Copy link
Copy Markdown
Contributor

QualOps Code Quality Analysis

Status: ⚠️ FAILED - Critical or high severity issues found

Summary

  • Total Issues: 29
  • Critical: 0 🔴
  • High: 5 🟠
  • Medium: 12 🟡
  • Low: 12 🟢
  • Files Analyzed: 19

🟠 High Issues (5)

  • sdk/eggai/adapters/a2a/executor.py:107 - security
    The string representation of every unhandled exception is forwarded to the A2A caller, potentially leaking connection strings, file paths, Pydantic field values, and other internal state.

  • sdk/eggai/channel.py:104 - bug
    Transport is subscribed before the connection is established, which may cause the subscription to fail or behave incorrectly if the transport requires an active connection first.

  • sdk/eggai/hooks.py:87 - bug
    Lambda in signal handler captures sig by closure incorrectly — always passes the last value of sig instead of the loop variable.

  • sdk/eggai/hooks.py:90 - bug
    Same closure bug in the fallback signal.signal handler — sig is captured by reference, not by value.

  • sdk/eggai/tracing.py:137 - bug
    When a provider is already configured, the span_exporter and BatchSpanProcessor are created but never used, yet the provider variable is silently replaced with the existing one without adding the processor.

🟡 Medium Issues (12)

  • sdk/eggai/adapters/a2a/plugin.py:124 - security
    The A2A server binds to 0.0.0.0 by default, exposing it on all network interfaces including external ones.

  • sdk/eggai/adapters/a2a/executor.py:36 - security
    All registered skill names are sent to the A2A caller in error responses, enabling full capability enumeration by any unauthenticated requester.

  • sdk/eggai/adapters/a2a/executor.py:69 - bug
    When data_type has __args__ but validation fails, the code still attempts to instantiate data_type(source=..., type=..., data=validated_data) with the raw dict, which may raise a type error if the constructor expects a validated model instance.

...and 9 more medium issues

📊 Full Report

View detailed report


Powered by QualOps

rocky-jaiswal and others added 7 commits September 7, 2026 16:00
- schemas.py: take main's required BaseMessage.data (#259), the type: ignore
  is no longer needed
- pyproject/poetry.lock: keep main's lock (#265) and only add mypy
- pending_reclaimer.py: annotate the xrange/xclaim results as Any; redis-py
  8.1 types them as unions the code already handles at runtime
- CHANGELOG: fold the PR entries into the current Unreleased sections
@pontino

pontino commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Maintainer rebase onto main (which now contains #259, #262, #265, #266, #267, #268, #269). Changes on top of your commits, kept minimal:

Verified locally with live Redis and Kafka: mypy clean in the CI configuration (no extras), 108 tests passed, ruff clean. Note for later: with the a2a and otel extras installed mypy reports 18 further errors in tracing.py and the a2a adapter; the lint job does not install extras so those are out of scope for now, and the pyproject says so.

Thanks for the two real fixes in here (the non-existent json.JSONEncodeError and the silent drop for a data_type without a type default).

@pontino
pontino force-pushed the chore/adopt-mypy-and-minor-fixes branch from cc88626 to 486dcec Compare September 7, 2026 14:02
@pontino
pontino merged commit 7edec20 into main Sep 7, 2026
12 of 13 checks passed
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