UAT: ContextService.list_files() calls get_container() internally - DI violation #3903

New issue

Open

opened 2026-04-06 07:19:50 +00:00 by freemo · 0 comments

freemo commented

2026-04-06 07:19:50 +00:00

Owner

Metadata

Branch: fix/context-service-di-violation
Commit Message: fix(context): remove Service Locator anti-pattern from ContextService.list_files()
Milestone: None (backlog — see note below)
Parent Epic: #396

Subtasks

Write a failing behave unit test that injects a mock ProjectService into ContextService and verifies list_files() uses it (TDD — test first)
Add project_service: ProjectService | None = None parameter to ContextService.__init__()
Store self._project_service = project_service in the constructor
Refactor list_files() to use self._project_service instead of calling get_container() internally
Update the DI container registration to pass project_service=project_service when constructing ContextService
Remove the from cleveragents.application.container import get_container import from list_files()
Verify nox -e typecheck passes (pyright)
Verify nox -e unit_tests passes with coverage >= 97%
Update PR description and link to this issue

Definition of Done

ContextService.__init__() accepts project_service as an injected dependency
list_files() no longer calls get_container() or imports from cleveragents.application.container
A behave unit test verifies the injected ProjectService mock is used by list_files()
DI container registration updated to wire project_service into ContextService
All nox stages pass
Coverage >= 97%

Bug Report

Feature Area: Dependency Injection and Service Layer

Severity: High — violates the Dependency Inversion Principle and makes the service untestable in isolation

What Was Tested

Code-level analysis of src/cleveragents/application/services/context_service.py against the specification's clean architecture and DI requirements.

Expected Behavior (from spec)

Per the specification and CONTRIBUTING.md, the project requires Dependency Injection as the standard mechanism for wiring services. Services must receive their dependencies through their constructor (constructor injection), not by calling get_container() internally. This is required for testability and inversion of control.

The ContextService is registered in the DI container and receives its dependencies via constructor injection. It should NOT call back into the container.

Actual Behavior

ContextService.list_files() (line 515 in context_service.py) calls get_container() internally when no project argument is provided:

def list_files(self, project: Project | None = None) -> list[str]:
    if project is None:
        # Import here to avoid circular dependency
        from cleveragents.application.container import get_container
        try:
            container = get_container()
            project_service = container.project_service()
            project = project_service.get_current_project()
            ...

This is a Service Locator anti-pattern — the service is reaching into the global container to resolve its own dependencies at runtime, bypassing the DI framework entirely.

Impact

Untestable in isolation: Unit tests cannot inject a mock ProjectService because the service resolves it at call time via the global container.
Circular dependency risk: ContextService → get_container() → ProjectService → UnitOfWork creates a hidden dependency chain not visible in the constructor signature.
Violates DI principle: The spec explicitly requires DI for testability and inversion of control.

Steps to Reproduce (Code Analysis)

Open src/cleveragents/application/services/context_service.py
Navigate to the list_files() method (line ~512)
Observe the from cleveragents.application.container import get_container import and get_container() call inside the method body

Code Location

File: src/cleveragents/application/services/context_service.py
Method: list_files(), lines ~512-525

Recommended Fix

Inject ProjectService into ContextService.__init__() as an optional dependency:

def __init__(
    self,
    settings: Settings,
    unit_of_work: UnitOfWork,
    *,
    vector_store_service: VectorStoreProtocol | None = None,
    event_bus: EventBus | None = None,
    project_service: ProjectService | None = None,  # ADD THIS
) -> None:
    ...
    self._project_service = project_service

Then update the container registration to pass project_service=project_service.

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-uat-tester

Backlog note: This issue was discovered during autonomous operation
on milestone v3.4.0 (ACMS v1 + Context Scaling). It does not block milestone completion and has been
placed in the backlog for human review and future milestone assignment.

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/context-service-di-violation` - **Commit Message**: `fix(context): remove Service Locator anti-pattern from ContextService.list_files()` - **Milestone**: None (backlog — see note below) - **Parent Epic**: #396 ## Subtasks - [ ] Write a failing `behave` unit test that injects a mock `ProjectService` into `ContextService` and verifies `list_files()` uses it (TDD — test first) - [ ] Add `project_service: ProjectService | None = None` parameter to `ContextService.__init__()` - [ ] Store `self._project_service = project_service` in the constructor - [ ] Refactor `list_files()` to use `self._project_service` instead of calling `get_container()` internally - [ ] Update the DI container registration to pass `project_service=project_service` when constructing `ContextService` - [ ] Remove the `from cleveragents.application.container import get_container` import from `list_files()` - [ ] Verify `nox -e typecheck` passes (pyright) - [ ] Verify `nox -e unit_tests` passes with coverage >= 97% - [ ] Update PR description and link to this issue ## Definition of Done - [ ] `ContextService.__init__()` accepts `project_service` as an injected dependency - [ ] `list_files()` no longer calls `get_container()` or imports from `cleveragents.application.container` - [ ] A `behave` unit test verifies the injected `ProjectService` mock is used by `list_files()` - [ ] DI container registration updated to wire `project_service` into `ContextService` - [ ] All nox stages pass - [ ] Coverage >= 97% --- ## Bug Report **Feature Area:** Dependency Injection and Service Layer **Severity:** High — violates the Dependency Inversion Principle and makes the service untestable in isolation ### What Was Tested Code-level analysis of `src/cleveragents/application/services/context_service.py` against the specification's clean architecture and DI requirements. ### Expected Behavior (from spec) Per the specification and CONTRIBUTING.md, the project requires **Dependency Injection** as the standard mechanism for wiring services. Services must receive their dependencies through their constructor (constructor injection), not by calling `get_container()` internally. This is required for testability and inversion of control. The `ContextService` is registered in the DI container and receives its dependencies via constructor injection. It should NOT call back into the container. ### Actual Behavior `ContextService.list_files()` (line 515 in `context_service.py`) calls `get_container()` internally when no `project` argument is provided: ```python def list_files(self, project: Project | None = None) -> list[str]: if project is None: # Import here to avoid circular dependency from cleveragents.application.container import get_container try: container = get_container() project_service = container.project_service() project = project_service.get_current_project() ... ``` This is a **Service Locator anti-pattern** — the service is reaching into the global container to resolve its own dependencies at runtime, bypassing the DI framework entirely. ### Impact 1. **Untestable in isolation**: Unit tests cannot inject a mock `ProjectService` because the service resolves it at call time via the global container. 2. **Circular dependency risk**: `ContextService` → `get_container()` → `ProjectService` → `UnitOfWork` creates a hidden dependency chain not visible in the constructor signature. 3. **Violates DI principle**: The spec explicitly requires DI for testability and inversion of control. ### Steps to Reproduce (Code Analysis) 1. Open `src/cleveragents/application/services/context_service.py` 2. Navigate to the `list_files()` method (line ~512) 3. Observe the `from cleveragents.application.container import get_container` import and `get_container()` call inside the method body ### Code Location - File: `src/cleveragents/application/services/context_service.py` - Method: `list_files()`, lines ~512-525 ### Recommended Fix Inject `ProjectService` into `ContextService.__init__()` as an optional dependency: ```python def __init__( self, settings: Settings, unit_of_work: UnitOfWork, *, vector_store_service: VectorStoreProtocol | None = None, event_bus: EventBus | None = None, project_service: ProjectService | None = None, # ADD THIS ) -> None: ... self._project_service = project_service ``` Then update the container registration to pass `project_service=project_service`. --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-uat-tester --- > **Backlog note:** This issue was discovered during autonomous operation > on milestone v3.4.0 (ACMS v1 + Context Scaling). It does not block milestone completion and has been > placed in the backlog for human review and future milestone assignment. --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator

freemo added a new dependency

2026-04-06 07:20:05 +00:00

#396 Epic: ACMS Context Pipeline

freemo referenced this issue

2026-04-06 07:20:44 +00:00

[Automated] CleverAgents Build Session - 2026-04-06 #3775

freemo referenced this issue

2026-04-06 07:39:33 +00:00

[Automated] CleverAgents Build Session - 2026-04-06 #3775

freemo referenced this issue

2026-04-06 07:49:16 +00:00

[Automated] CleverAgents Build Session - 2026-04-06 #3775