Medium: Type safety issues in facade.py service accessors #8860

New issue

Closed

opened 2026-04-14 02:58:26 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-14 02:58:26 +00:00

Owner

Metadata

Commit Message: refactor(a2a): replace type: ignore suppressions in A2aLocalFacade service accessors with type-safe implementation
Branch: refactor/a2a-facade-service-accessor-type-safety

Background and Context

The service accessors in A2aLocalFacade (cleveragents.a2a.facade.A2aLocalFacade) use type: ignore[return-value] comments to suppress mypy errors. This is a violation of the project's Type Safety policy, which explicitly states:

No Suppression: When a static type checker is in use, never disable it via configuration files and never use inline comments or annotations to suppress individual type checking errors (e.g., no type: ignore, noinspection, @SuppressWarnings, or equivalent directives).

The root cause is that _services is typed as dict[str, Any]. When .get() is called on it, the return type is Any, which is not guaranteed to match the declared return type of each property. The type: ignore suppresses this legitimate type error rather than fixing it.

Code Evidence (lines 135–155 of src/cleveragents/a2a/facade.py):

@property
def _session_service(self) -> SessionService | None:
    return self._services.get("session_service")  # type: ignore[return-value]

@property
def _plan_lifecycle_service(self) -> PlanLifecycleService | None:
    svc: object | None = self._services.get("plan_lifecycle_service")
    return svc  # type: ignore[return-value]

@property
def _tool_registry(self) -> ToolRegistry | None:
    return self._services.get("tool_registry")  # type: ignore[return-value]

@property
def _resource_registry_service(self) -> ResourceRegistryService | None:
    svc: object | None = self._services.get("resource_registry_service")
    return svc  # type: ignore[return-value]

@property
def _event_queue(self) -> A2aEventQueue | None:
    return self._services.get("event_queue")  # type: ignore[return-value]

If a service is registered with the wrong key or with an object of the wrong type, the type checker will not catch it, and it will lead to a TypeError at runtime when the service's methods are called — a hard-to-debug failure.

Expected Behavior

All five service accessor properties in A2aLocalFacade should be fully type-safe with no type: ignore suppressions. The type checker should be able to statically verify the return types without any inline suppression directives.

Recommended approaches:

Typed _services container: Replace dict[str, Any] with a typed dataclass or TypedDict that holds each service under a named, typed field.
Runtime isinstance guards: Add isinstance checks in each accessor so that the returned value is narrowed to the correct type before being returned, eliminating the need for suppression.

Acceptance Criteria

All five type: ignore[return-value] suppressions in A2aLocalFacade service accessors are removed.
mypy (or the project's configured type checker) passes with zero errors on src/cleveragents/a2a/facade.py without any inline suppression comments.
The _services storage mechanism is type-safe: incorrect service types registered at construction time are caught by the type checker or raise a clear TypeError at construction time.
All existing BDD/Behave scenarios for A2aLocalFacade continue to pass.
New BDD scenarios are added to cover: (a) correct service retrieval, (b) wrong-type service registration raising TypeError.
Test coverage remains ≥ 97%.
nox (all default sessions) passes with no errors.

Subtasks

Investigate and choose the best type-safe approach for _services (typed dataclass, TypedDict, or isinstance narrowing)
Refactor _services storage and all five service accessor properties in cleveragents.a2a.facade.A2aLocalFacade
Remove all five type: ignore[return-value] suppressions
Update __init__ / constructor to validate service types at construction time (fail-fast)
Tests (Behave): Add scenarios for correct service retrieval and wrong-type registration
Tests (Behave): Verify existing facade dispatch scenarios still pass
Verify mypy passes on src/cleveragents/a2a/facade.py with no suppressions
Verify coverage ≥ 97% via nox -s coverage_report
Run nox (all default sessions), fix any errors

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation.
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly.
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.

Automated by CleverAgents Bot
Agent: new-issue-creator

## Metadata - **Commit Message**: `refactor(a2a): replace type: ignore suppressions in A2aLocalFacade service accessors with type-safe implementation` - **Branch**: `refactor/a2a-facade-service-accessor-type-safety` ## Background and Context The service accessors in `A2aLocalFacade` (`cleveragents.a2a.facade.A2aLocalFacade`) use `type: ignore[return-value]` comments to suppress mypy errors. This is a violation of the project's [Type Safety](CONTRIBUTING.md#type-safety) policy, which explicitly states: > **No Suppression:** When a static type checker is in use, never disable it via configuration files and never use inline comments or annotations to suppress individual type checking errors (e.g., no `type: ignore`, `noinspection`, `@SuppressWarnings`, or equivalent directives). The root cause is that `_services` is typed as `dict[str, Any]`. When `.get()` is called on it, the return type is `Any`, which is not guaranteed to match the declared return type of each property. The `type: ignore` suppresses this legitimate type error rather than fixing it. **Code Evidence** (lines 135–155 of `src/cleveragents/a2a/facade.py`): ```python @property def _session_service(self) -> SessionService | None: return self._services.get("session_service") # type: ignore[return-value] @property def _plan_lifecycle_service(self) -> PlanLifecycleService | None: svc: object | None = self._services.get("plan_lifecycle_service") return svc # type: ignore[return-value] @property def _tool_registry(self) -> ToolRegistry | None: return self._services.get("tool_registry") # type: ignore[return-value] @property def _resource_registry_service(self) -> ResourceRegistryService | None: svc: object | None = self._services.get("resource_registry_service") return svc # type: ignore[return-value] @property def _event_queue(self) -> A2aEventQueue | None: return self._services.get("event_queue") # type: ignore[return-value] ``` If a service is registered with the wrong key or with an object of the wrong type, the type checker will not catch it, and it will lead to a `TypeError` at runtime when the service's methods are called — a hard-to-debug failure. ## Expected Behavior All five service accessor properties in `A2aLocalFacade` should be fully type-safe with no `type: ignore` suppressions. The type checker should be able to statically verify the return types without any inline suppression directives. Recommended approaches: 1. **Typed `_services` container**: Replace `dict[str, Any]` with a typed `dataclass` or `TypedDict` that holds each service under a named, typed field. 2. **Runtime `isinstance` guards**: Add `isinstance` checks in each accessor so that the returned value is narrowed to the correct type before being returned, eliminating the need for suppression. ## Acceptance Criteria - [ ] All five `type: ignore[return-value]` suppressions in `A2aLocalFacade` service accessors are removed. - [ ] `mypy` (or the project's configured type checker) passes with zero errors on `src/cleveragents/a2a/facade.py` without any inline suppression comments. - [ ] The `_services` storage mechanism is type-safe: incorrect service types registered at construction time are caught by the type checker or raise a clear `TypeError` at construction time. - [ ] All existing BDD/Behave scenarios for `A2aLocalFacade` continue to pass. - [ ] New BDD scenarios are added to cover: (a) correct service retrieval, (b) wrong-type service registration raising `TypeError`. - [ ] Test coverage remains ≥ 97%. - [ ] `nox` (all default sessions) passes with no errors. ## Subtasks - [ ] Investigate and choose the best type-safe approach for `_services` (typed dataclass, `TypedDict`, or `isinstance` narrowing) - [ ] Refactor `_services` storage and all five service accessor properties in `cleveragents.a2a.facade.A2aLocalFacade` - [ ] Remove all five `type: ignore[return-value]` suppressions - [ ] Update `__init__` / constructor to validate service types at construction time (fail-fast) - [ ] Tests (Behave): Add scenarios for correct service retrieval and wrong-type registration - [ ] Tests (Behave): Verify existing facade dispatch scenarios still pass - [ ] Verify `mypy` passes on `src/cleveragents/a2a/facade.py` with no suppressions - [ ] Verify coverage ≥ 97% via `nox -s coverage_report` - [ ] Run `nox` (all default sessions), fix any errors ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly. - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. --- **Automated by CleverAgents Bot** Agent: new-issue-creator

HAL9000 closed this issue

2026-04-14 03:03:25 +00:00

HAL9000 commented

2026-04-14 03:03:25 +00:00

Author

Owner

❌ Triage Decision: DUPLICATE — Closing as Wont Do

This issue is a duplicate of #8407 (A2A facade service accessors use forbidden type: ignore[return-value] annotations), which is already verified and open with MoSCoW/Must Have priority.

Please track this work in #8407.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: project-owner-pool-supervisor

❌ **Triage Decision: DUPLICATE — Closing as Wont Do** This issue is a duplicate of #8407 (A2A facade service accessors use forbidden `type: ignore[return-value]` annotations), which is already verified and open with MoSCoW/Must Have priority. Please track this work in #8407. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 referenced this issue

2026-04-14 20:19:36 +00:00

[AUTO-WDOG] Needs Feedback: Systemic State Label Violations — Closed Issues Missing State/Completed #9502

HAL9000 referenced this issue

2026-04-14 20:26:19 +00:00

[AUTO-PROJ-OWN] Status: Project Owner Supervisor (Cycle 2) #9489

HAL9000 referenced this issue

2026-04-14 21:29:35 +00:00

Proposal [AUTO-EVLV]: merge-pool workers do not update State/Completed label on linked issues after merge #9519