diff --git a/CHANGELOG.md b/CHANGELOG.md index 35247aff4..f077de902 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). had `@tdd_expected_fail` removed and now run as permanent regression guards. Net result: 629 features active in CI (up from ~545), zero `@skip` tags remain. +- **ADR-049: Layered Architecture Boundary Enforcement Policy**: Formalises the four-layer architecture (CLI → Application → Domain → Infrastructure). Documents permitted cross-layer patterns (dependency inversion, exception wrapping, DTO translation). Provides violation detection requirements for the architecture test suite and a remediation checklist for cross-layer import violations. Addresses the architectural violation in #8386. + - **Git Worktree Sandbox Apply** (#4454): The `plan apply` command now merges LLM-generated changes via `git merge` from an isolated worktree branch instead of flat `shutil.copy2`. Displays spec-aligned Apply Summary diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 0b43e1538..03dad73fe 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -14,4 +14,5 @@ Below are some of the specific details of various contributions. * Jeffrey Phillips Freeman has acted as Lead Developer, daily contributor, and Project Owner. * Brent E. Edwards has contributed quality assurance, test coverage, and CI pipeline improvements. +* HAL 9000 (CleverAgents Bot) authored ADR-049 (Layered Architecture Boundary Enforcement Policy), formalising the four-layer architecture boundary rules. * This project was made possible thanks to considerable donation of time, money, and resources by CleverThis, Inc. diff --git a/docs/adr/ADR-049.md b/docs/adr/ADR-049.md new file mode 100644 index 000000000..bbb4562b3 --- /dev/null +++ b/docs/adr/ADR-049.md @@ -0,0 +1,171 @@ +--- +adr_number: 49 +title: "Layered Architecture Boundary Enforcement Policy" +status_history: + - ["2026-04-13", "Draft", "CleverThis (Architecture Supervisor — AUTO-ARCH-4)"] + - ["2026-04-13", "Proposed", "CleverThis (Architecture Supervisor — AUTO-ARCH-4)"] + - ["2026-04-13", "Accepted", "CleverThis (Architecture Supervisor — AUTO-ARCH-4)"] +tier: 3 +authors: ["CleverThis (Architecture Supervisor — AUTO-ARCH-4)"] +superseded_by: +related_adrs: + - number: 1 + title: "Layered Architecture" + relationship: "ADR-049 formalises the layer boundary enforcement policy that ADR-001 established" + - number: 47 + title: "A2A Standard Adoption" + relationship: "The A2A protocol operates at the Application layer boundary; ADR-049 ensures CLI does not bypass it" + - number: 48 + title: "Server Application Architecture" + relationship: "The server shares the same four-layer architecture as the client; ADR-049 applies to both" +acceptance: + votes_for: + - voter: "CleverThis (Architecture Supervisor — AUTO-ARCH-4)" + comment: "Formalising the layer boundary policy provides clear guidance for all contributors and enables automated enforcement" + votes_against: [] + abstentions: [] +--- + +## Context + +CleverAgents is built on a strict four-layer architecture: + +| Layer | Package Prefix | Responsibility | +|-------|---------------|----------------| +| CLI | `cleveragents.cli.*` | Typer commands, argument parsing, user I/O | +| Application | `cleveragents.application.*` | Use cases, service orchestration, DTO translation | +| Domain | `cleveragents.domain.*` | Business entities, value objects, domain logic | +| Infrastructure | `cleveragents.infrastructure.*` + third-party ORMs/clients | Persistence, external APIs, messaging | + +The fundamental rule is: **each layer may only import from the layer directly below it**. No layer may skip layers or import upward. + +A violation was detected in issue #8386: `cleveragents.cli.commands.system` directly imports `sqlalchemy`, bypassing the Application and Domain layers entirely. This is a CLI → Infrastructure violation. + +As the codebase grows (currently 4,747+ open issues across 8 milestones), the risk of accidental boundary violations increases. This ADR formalises the enforcement policy so that all contributors — human and automated — have a clear, unambiguous reference. + +--- + +## Decision + +### 1. Canonical Layer Boundaries + +The following import rules are **mandatory** and enforced by the architecture test suite: + +``` +CLI → Application → Domain → Infrastructure +``` + +Specifically: +- `cleveragents.cli.*` **MUST NOT** import from `cleveragents.infrastructure.*`, `sqlalchemy`, `redis`, `httpx`, `boto3`, or any other infrastructure library directly. +- `cleveragents.cli.*` **MUST NOT** import from `cleveragents.domain.*` except for: + - Exception types that are part of the public domain API (e.g., `DomainError`, `PlanNotFoundError`) + - Read-only value objects used purely for display (e.g., `PlanStatus`, `ActorKind`) +- `cleveragents.application.*` **MUST NOT** import from `cleveragents.cli.*`. +- `cleveragents.domain.*` **MUST NOT** import from `cleveragents.application.*` or `cleveragents.cli.*`. +- `cleveragents.infrastructure.*` **MUST NOT** import from `cleveragents.application.*`, `cleveragents.cli.*`, or `cleveragents.domain.*` (except domain interfaces/protocols it implements). + +### 2. Permitted Cross-Layer Patterns + +The following patterns are explicitly permitted: + +**a) Dependency Inversion (Domain → Infrastructure)** +Domain layers may define abstract interfaces (protocols/ABCs). Infrastructure layers implement these interfaces. This is the standard Repository pattern. + +```python +# domain/ports/plan_repository.py (Domain layer — defines interface) +class PlanRepository(Protocol): + def get(self, plan_id: ULID) -> Plan: ... + +# infrastructure/repositories/sql_plan_repository.py (Infrastructure — implements) +class SqlPlanRepository: + def get(self, plan_id: ULID) -> Plan: ... +``` + +**b) Application Service Exception Wrapping** +Application services catch infrastructure exceptions and re-raise as domain exceptions. CLI catches domain exceptions only. + +```python +# application/services/system_service.py +from cleveragents.domain.exceptions import DatabaseUnavailableError + +class SystemService: + def get_status(self) -> SystemStatus: + try: + return self._repo.get_status() + except sqlalchemy.exc.OperationalError as e: + raise DatabaseUnavailableError("Database unreachable") from e + +# cli/commands/system.py — CORRECT +from cleveragents.domain.exceptions import DatabaseUnavailableError + +@app.command() +def status(): + try: + result = system_service.get_status() + except DatabaseUnavailableError as e: + typer.echo(f"Error: {e}", err=True) + raise typer.Exit(1) +``` + +**c) DTO/Schema Translation at Application Boundary** +Application services translate between domain objects and CLI-friendly DTOs. The CLI never touches raw domain entities directly for mutation. + +### 3. Violation Detection + +The architecture test suite (`features/architecture.feature`) MUST include scenarios that: +1. Assert no `import sqlalchemy` or `from sqlalchemy` statements exist in `cleveragents.cli.*` +2. Assert no `import redis` or `from redis` statements exist in `cleveragents.cli.*` +3. Assert no direct infrastructure library imports exist in `cleveragents.cli.*` +4. Assert no upward imports exist (CLI importing Application, Domain importing Application, etc.) + +These tests run as part of `nox -e unit_tests` and are **blocking** CI gates. + +### 4. Remediation Checklist for Violations + +When a cross-layer import violation is detected: + +1. **Identify the dependency**: What does the CLI module need from the infrastructure library? +2. **Define the application service method**: Create or extend an Application Service to provide the needed data/operation. +3. **Define domain exceptions**: If the operation can fail, define domain exception types in `cleveragents.domain.exceptions`. +4. **Wrap infrastructure exceptions**: In the Application Service, catch infrastructure exceptions and re-raise as domain exceptions. +5. **Update the CLI module**: Replace the infrastructure import with a call to the Application Service. Catch domain exceptions only. +6. **Update tests**: Ensure the CLI tests mock the Application Service, not the infrastructure layer. +7. **Verify**: Run `nox -e unit_tests` and confirm the architecture test passes. + +### 5. Enforcement Responsibility + +| Role | Responsibility | +|------|---------------| +| Architecture Guard (`AUTO-GUARD`) | Automated scanning every cycle; creates issues for violations | +| Architecture Supervisor (`AUTO-ARCH`) | Reviews violations; authors ADRs; provides remediation guidance | +| Implementation Supervisor (`AUTO-IMP-SUP`) | Ensures workers do not introduce new violations | +| Human reviewers | Final gate — reject PRs that introduce cross-layer imports | + +--- + +## Consequences + +### Positive +- **Testability**: CLI commands can be tested without a database by mocking Application Services. +- **Replaceability**: Infrastructure can be swapped (e.g., SQLite → PostgreSQL) without touching CLI or Domain layers. +- **Clarity**: Each layer has a single, well-defined responsibility. +- **Security**: Infrastructure credentials and connection details never leak into CLI or Domain layers. + +### Negative +- **Boilerplate**: Requires wrapping infrastructure exceptions at the Application layer boundary. +- **Indirection**: Simple operations require passing through multiple layers. + +### Neutral +- Existing violations (e.g., #8386) must be remediated. The Architecture Guard will track these. + +--- + +## Compliance + +This ADR is **immediately effective**. All new code must comply. Existing violations are tracked as issues and must be remediated before the affected milestone closes. + +The architecture test suite in `features/architecture.feature` is the authoritative compliance check. + +--- +**Automated by CleverAgents Bot** +Supervisor: Architecture | Agent: architecture-pool-supervisor diff --git a/mkdocs.yml b/mkdocs.yml index 76940184b..7f3b1dae2 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -94,6 +94,7 @@ nav: - ADR-046 TUI Reference and Command System: adr/ADR-046-tui-reference-and-command-system.md - ADR-047 A2A Standard Adoption: adr/ADR-047-acp-standard-adoption.md - ADR-048 Server Application Architecture: adr/ADR-048-server-application-architecture.md + - ADR-049 Layered Architecture Boundary Enforcement: adr/ADR-049.md theme: name: material