UAT: PermissionService uses module-level singleton pattern instead of DI container registration #3932

Open
opened 2026-04-06 07:35:12 +00:00 by freemo · 0 comments
Owner

Metadata

  • Branch: fix/permission-service-di-container-registration
  • Commit Message: fix(container): register PermissionService in DI container and remove module-level singleton
  • Milestone: None (backlog — see note below)
  • Parent Epic: #397

Backlog note: This issue was discovered during autonomous operation
on milestone v3.6.0. It does not block milestone completion and has been
placed in the backlog for human review and future milestone assignment.

Bug Report

Feature Area: Dependency Injection and Service Layer

Severity: Medium — PermissionService bypasses the DI container using a module-level singleton, making it impossible to override in tests via the standard container mechanism

What Was Tested

Code-level analysis of src/cleveragents/application/services/permission_service.py against DI requirements.

Expected Behavior (from spec)

Per the specification and CONTRIBUTING.md, Dependency Injection is the required pattern for managing service instances. Services should be registered in the DI container (Container class) and resolved via container.permission_service(). This allows test overrides via container.override_providers().

Actual Behavior

permission_service.py implements a module-level singleton pattern that bypasses the DI container entirely:

# Lines ~295-325 in permission_service.py
_default_service: PermissionService | None = None

def get_default_permission_service() -> PermissionService:
    """Return the module-level default PermissionService."""
    global _default_service
    if _default_service is None:
        _default_service = PermissionService()
    return _default_service

def set_default_permission_service(service: PermissionService | None) -> None:
    """Replace (or clear) the module-level default PermissionService."""
    global _default_service
    _default_service = service

This pattern:

  1. Creates a global mutable singleton at the module level
  2. Provides a set_default_permission_service() function for test overrides (instead of using the DI container)
  3. Is NOT registered in the Container class in container.py

Impact

  1. DI bypass: The PermissionService is not resolvable via container.permission_service() — it doesn't exist in the container.
  2. Global mutable state: Module-level singletons create shared state between tests, causing test pollution.
  3. Non-standard override mechanism: Tests must call set_default_permission_service() instead of using container.override_providers(), creating two different override mechanisms.
  4. Spec violation: The spec requires DI for all services.

Steps to Reproduce (Code Analysis)

  1. Open src/cleveragents/application/services/permission_service.py
  2. Search for _default_service, get_default_permission_service, set_default_permission_service
  3. Open src/cleveragents/application/container.py and search for PermissionService — it is not registered

Code Locations

  • File: src/cleveragents/application/services/permission_service.py
  • Lines: ~295-325 (module-level singleton functions)
  • Container: src/cleveragents/application/container.py (missing permission_service provider)
  1. Remove the module-level singleton (_default_service, get_default_permission_service, set_default_permission_service).
  2. Register PermissionService in the DI container:
# In Container class:
permission_service = providers.Singleton(
    PermissionService,
)
  1. Update callers to use container.permission_service() instead of get_default_permission_service().

Subtasks

  • Remove _default_service, get_default_permission_service, and set_default_permission_service from permission_service.py
  • Register PermissionService as a providers.Singleton in container.py
  • Audit all callers of get_default_permission_service() and update them to use container.permission_service()
  • Audit all callers of set_default_permission_service() in tests and replace with container.override_providers()
  • Run nox -s lint and nox -s typecheck to verify no regressions
  • Verify coverage remains ≥ 97% via nox -s coverage_report
  • Run full nox suite and confirm all stages pass

Definition of Done

  • PermissionService is registered in the DI container and resolvable via container.permission_service()
  • No module-level singleton pattern (_default_service, get_default_permission_service, set_default_permission_service) remains in permission_service.py
  • All callers use the DI container for resolution and container.override_providers() for test overrides
  • All nox stages pass
  • Coverage >= 97%
  • PR merged to master

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

## Metadata - **Branch**: `fix/permission-service-di-container-registration` - **Commit Message**: `fix(container): register PermissionService in DI container and remove module-level singleton` - **Milestone**: None (backlog — see note below) - **Parent Epic**: #397 > **Backlog note:** This issue was discovered during autonomous operation > on milestone v3.6.0. It does not block milestone completion and has been > placed in the backlog for human review and future milestone assignment. ## Bug Report **Feature Area:** Dependency Injection and Service Layer **Severity:** Medium — `PermissionService` bypasses the DI container using a module-level singleton, making it impossible to override in tests via the standard container mechanism ### What Was Tested Code-level analysis of `src/cleveragents/application/services/permission_service.py` against DI requirements. ### Expected Behavior (from spec) Per the specification and CONTRIBUTING.md, **Dependency Injection** is the required pattern for managing service instances. Services should be registered in the DI container (`Container` class) and resolved via `container.permission_service()`. This allows test overrides via `container.override_providers()`. ### Actual Behavior `permission_service.py` implements a module-level singleton pattern that bypasses the DI container entirely: ```python # Lines ~295-325 in permission_service.py _default_service: PermissionService | None = None def get_default_permission_service() -> PermissionService: """Return the module-level default PermissionService.""" global _default_service if _default_service is None: _default_service = PermissionService() return _default_service def set_default_permission_service(service: PermissionService | None) -> None: """Replace (or clear) the module-level default PermissionService.""" global _default_service _default_service = service ``` This pattern: 1. Creates a global mutable singleton at the module level 2. Provides a `set_default_permission_service()` function for test overrides (instead of using the DI container) 3. Is NOT registered in the `Container` class in `container.py` ### Impact 1. **DI bypass**: The `PermissionService` is not resolvable via `container.permission_service()` — it doesn't exist in the container. 2. **Global mutable state**: Module-level singletons create shared state between tests, causing test pollution. 3. **Non-standard override mechanism**: Tests must call `set_default_permission_service()` instead of using `container.override_providers()`, creating two different override mechanisms. 4. **Spec violation**: The spec requires DI for all services. ### Steps to Reproduce (Code Analysis) 1. Open `src/cleveragents/application/services/permission_service.py` 2. Search for `_default_service`, `get_default_permission_service`, `set_default_permission_service` 3. Open `src/cleveragents/application/container.py` and search for `PermissionService` — it is not registered ### Code Locations - File: `src/cleveragents/application/services/permission_service.py` - Lines: ~295-325 (module-level singleton functions) - Container: `src/cleveragents/application/container.py` (missing `permission_service` provider) ### Recommended Fix 1. Remove the module-level singleton (`_default_service`, `get_default_permission_service`, `set_default_permission_service`). 2. Register `PermissionService` in the DI container: ```python # In Container class: permission_service = providers.Singleton( PermissionService, ) ``` 3. Update callers to use `container.permission_service()` instead of `get_default_permission_service()`. ## Subtasks - [ ] Remove `_default_service`, `get_default_permission_service`, and `set_default_permission_service` from `permission_service.py` - [ ] Register `PermissionService` as a `providers.Singleton` in `container.py` - [ ] Audit all callers of `get_default_permission_service()` and update them to use `container.permission_service()` - [ ] Audit all callers of `set_default_permission_service()` in tests and replace with `container.override_providers()` - [ ] Run `nox -s lint` and `nox -s typecheck` to verify no regressions - [ ] Verify coverage remains ≥ 97% via `nox -s coverage_report` - [ ] Run full `nox` suite and confirm all stages pass ## Definition of Done - [ ] `PermissionService` is registered in the DI container and resolvable via `container.permission_service()` - [ ] No module-level singleton pattern (`_default_service`, `get_default_permission_service`, `set_default_permission_service`) remains in `permission_service.py` - [ ] All callers use the DI container for resolution and `container.override_providers()` for test overrides - [ ] All nox stages pass - [ ] Coverage >= 97% - [ ] PR merged to `master` --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Blocks
#397 Epic: Server & Autonomy Infrastructure
cleveragents/cleveragents-core
Reference
cleveragents/cleveragents-core#3932
No description provided.