cleveragents/cleveragents-core

Fork 3

phase_gating.py directly imports sqlalchemy.exc (architecture violation) #8439

New issue

Open

opened 2026-04-13 18:53:48 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-13 18:53:48 +00:00

Owner

Metadata

Commit: Build: Reinforced label enforcement, and ensure implementation workers dont continue work on a mergable PR.
Branch: main (master SHA: 5a9aaa79edaefb1a257114f054ea87facb8efe69)

Background and Context

src/cleveragents/application/services/phase_gating.py contains a direct import of sqlalchemy.exc.OperationalError at the application-services layer. This is the same cross-layer architecture violation as the known bug #8386 (cleveragents.cli.commands.system directly imports sqlalchemy), but in a different module. The application layer must not import infrastructure libraries directly; it should depend only on domain and core abstractions.

# phase_gating.py line ~12
from sqlalchemy.exc import OperationalError

This import is used in resolve_plan_phase() to catch database errors during phase lookup:

except (DatabaseError, OperationalError, OSError):
    logger.warning(...)

The OperationalError from SQLAlchemy is an infrastructure concern. The application layer should only catch the project's own DatabaseError abstraction (which is already caught in the same except clause), not raw SQLAlchemy exceptions.

Current Behavior

phase_gating.py imports from sqlalchemy.exc import OperationalError directly, creating a hard dependency on SQLAlchemy in the application services layer. This violates the layered architecture rule that application services must not import infrastructure libraries.

Expected Behavior

Per the project's architecture rules (same as #8386), the application layer must not directly import SQLAlchemy. The DatabaseError abstraction from cleveragents.core.exceptions should be sufficient to catch infrastructure-level database errors. The OperationalError catch should either be removed (relying solely on DatabaseError) or OperationalError should be wrapped/re-raised as DatabaseError at the infrastructure boundary.

Acceptance Criteria

phase_gating.py no longer imports from sqlalchemy.exc import OperationalError
The resolve_plan_phase() function still correctly handles database errors during phase lookup
pyright and ruff pass with no new violations
All existing BDD scenarios for decision_service and phase_gating continue to pass
No import sqlalchemy or from sqlalchemy appears in src/cleveragents/application/ (verified by architecture test)

Subtasks

Remove from sqlalchemy.exc import OperationalError from phase_gating.py
Verify that DatabaseError from cleveragents.core.exceptions is sufficient to catch the SQLAlchemy OperationalError at the infrastructure boundary (check the DB error wrapping layer)
If OperationalError is not wrapped, add wrapping in the infrastructure layer instead
Update or add architecture test to cover phase_gating.py
Run nox -s lint typecheck unit_tests to confirm no regressions

Definition of Done

This issue is closed when phase_gating.py contains no direct SQLAlchemy imports, the phase-gating logic still correctly handles DB errors, all tests pass, and the architecture test covers this module.

Automated by CleverAgents Bot
Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor

## Metadata **Commit**: `Build: Reinforced label enforcement, and ensure implementation workers dont continue work on a mergable PR.` **Branch**: `main` (master SHA: `5a9aaa79edaefb1a257114f054ea87facb8efe69`) ## Background and Context `src/cleveragents/application/services/phase_gating.py` contains a direct import of `sqlalchemy.exc.OperationalError` at the application-services layer. This is the same cross-layer architecture violation as the known bug #8386 (`cleveragents.cli.commands.system` directly imports sqlalchemy), but in a different module. The application layer must not import infrastructure libraries directly; it should depend only on domain and core abstractions. ```python # phase_gating.py line ~12 from sqlalchemy.exc import OperationalError ``` This import is used in `resolve_plan_phase()` to catch database errors during phase lookup: ```python except (DatabaseError, OperationalError, OSError): logger.warning(...) ``` The `OperationalError` from SQLAlchemy is an infrastructure concern. The application layer should only catch the project's own `DatabaseError` abstraction (which is already caught in the same except clause), not raw SQLAlchemy exceptions. ## Current Behavior `phase_gating.py` imports `from sqlalchemy.exc import OperationalError` directly, creating a hard dependency on SQLAlchemy in the application services layer. This violates the layered architecture rule that application services must not import infrastructure libraries. ## Expected Behavior Per the project's architecture rules (same as #8386), the application layer must not directly import SQLAlchemy. The `DatabaseError` abstraction from `cleveragents.core.exceptions` should be sufficient to catch infrastructure-level database errors. The `OperationalError` catch should either be removed (relying solely on `DatabaseError`) or `OperationalError` should be wrapped/re-raised as `DatabaseError` at the infrastructure boundary. ## Acceptance Criteria - [ ] `phase_gating.py` no longer imports `from sqlalchemy.exc import OperationalError` - [ ] The `resolve_plan_phase()` function still correctly handles database errors during phase lookup - [ ] `pyright` and `ruff` pass with no new violations - [ ] All existing BDD scenarios for `decision_service` and `phase_gating` continue to pass - [ ] No `import sqlalchemy` or `from sqlalchemy` appears in `src/cleveragents/application/` (verified by architecture test) ## Subtasks - [ ] Remove `from sqlalchemy.exc import OperationalError` from `phase_gating.py` - [ ] Verify that `DatabaseError` from `cleveragents.core.exceptions` is sufficient to catch the SQLAlchemy `OperationalError` at the infrastructure boundary (check the DB error wrapping layer) - [ ] If `OperationalError` is not wrapped, add wrapping in the infrastructure layer instead - [ ] Update or add architecture test to cover `phase_gating.py` - [ ] Run `nox -s lint typecheck unit_tests` to confirm no regressions ## Definition of Done This issue is closed when `phase_gating.py` contains no direct SQLAlchemy imports, the phase-gating logic still correctly handles DB errors, all tests pass, and the architecture test covers this module. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor

HAL9000 added the

labels

2026-04-13 19:02:00 +00:00

HAL9000 added this to the v3.2.0 milestone

2026-04-13 19:06:28 +00:00

HAL9000 modified the milestone from v3.2.0 to v3.5.0

2026-04-13 19:12:58 +00:00

HAL9000 added

and removed

labels

2026-04-13 20:37:13 +00:00

HAL9000 commented

2026-04-13 20:37:43 +00:00

Author

Owner

[AUTO-OWNR-5] Triage Decision

Status: ✅ Verified

MoSCoW: Must Have
Priority: High

Rationale: This is a confirmed architecture violation in phase_gating.py — a direct import of sqlalchemy.exc.OperationalError at the application-services layer. This directly violates ADR-049 (layered architecture enforcement), which mandates that the application layer must not import infrastructure libraries. The project already has a DatabaseError abstraction in cleveragents.core.exceptions for exactly this purpose. This is the same class of violation as the known bug #8386 and must be resolved to maintain architectural integrity for v3.5.0. Classified as Must Have because architecture violations in the application layer undermine the entire layered design and can propagate to other modules if left unaddressed.

Next Steps: Remove from sqlalchemy.exc import OperationalError from phase_gating.py. Verify that DatabaseError from cleveragents.core.exceptions is sufficient to catch the underlying SQLAlchemy error at the infrastructure boundary. If not, add wrapping in the infrastructure layer. Update or add an architecture test to cover this module. Run nox -s lint typecheck unit_tests to confirm no regressions.

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

## [AUTO-OWNR-5] Triage Decision **Status**: ✅ Verified **MoSCoW**: Must Have **Priority**: High **Rationale**: This is a confirmed architecture violation in `phase_gating.py` — a direct import of `sqlalchemy.exc.OperationalError` at the application-services layer. This directly violates ADR-049 (layered architecture enforcement), which mandates that the application layer must not import infrastructure libraries. The project already has a `DatabaseError` abstraction in `cleveragents.core.exceptions` for exactly this purpose. This is the same class of violation as the known bug #8386 and must be resolved to maintain architectural integrity for v3.5.0. Classified as **Must Have** because architecture violations in the application layer undermine the entire layered design and can propagate to other modules if left unaddressed. **Next Steps**: Remove `from sqlalchemy.exc import OperationalError` from `phase_gating.py`. Verify that `DatabaseError` from `cleveragents.core.exceptions` is sufficient to catch the underlying SQLAlchemy error at the infrastructure boundary. If not, add wrapping in the infrastructure layer. Update or add an architecture test to cover this module. Run `nox -s lint typecheck unit_tests` to confirm no regressions. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 referenced this issue

2026-04-13 20:40:04 +00:00

[AUTO-OWNR] Status: Project Owner Pool Supervisor (Cycle 1) #8518