Merge pull request #292 from finos/feat/architecture-cleanup

refactor: architecture remediation — dead code, layering, registry, concurrency, god-object decomposition

Author: fgogolli

.importlinter

@@ -0,0 +1,78 @@
+[importlinter]
+root_package = orb
+include_external_packages = True
+
+# ---------------------------------------------------------------------------
+# Contract 1: domain-is-clean
+#
+# The domain layer must not import from any outer layer or from CLI/UI
+# frameworks (argparse, etc.).  This is the strictest contract — any new
+# violation here is a clean-architecture regression.
+#
+# NOTE: allow_indirect_imports = True is required because import-linter
+# resolves the root `orb` package as an intermediate node when any domain
+# module does `from orb.domain...`.  The root orb/__init__.py re-exports
+# SDK/bootstrap symbols which transitively touch outer layers, creating
+# false positives on every domain file.  Direct-only enforcement is the
+# correct policy here.
+# ---------------------------------------------------------------------------
+[importlinter:contract:domain-is-clean]
+name = Domain layer is clean (no outer-layer imports)
+type = forbidden
+source_modules = orb.domain
+forbidden_modules =
+    orb.application
+    orb.infrastructure
+    orb.interface
+    orb.providers
+    orb.monitoring
+    orb.cli
+    argparse
+allow_indirect_imports = True
+# No baseline ignore_imports needed: the only domain argparse violation
+# (provider_cli_spec_port) was moved to orb.providers.base in this branch (E2).
+# Any future argparse-in-domain import is now a real CI failure.
+
+
+# ---------------------------------------------------------------------------
+# Contract 2: application-no-infra-concretes
+#
+# The application layer must not import from infrastructure concretes,
+# provider implementations, or monitoring adapters.  It may only depend on
+# domain (pure) and its own cross-cutting application interfaces.
+#
+# All A1/A2/A3 baseline violations have been fixed by Epic E4 / ticket #1358:
+#   A1 — TemplateDTO moved to orb.application.dto.template
+#   A2 — same fix as A1
+#   A3 — MetricsCollector now injected via MetricsPort
+# ---------------------------------------------------------------------------
+[importlinter:contract:application-no-infra-concretes]
+name = Application layer does not import infrastructure concretes
+type = forbidden
+source_modules = orb.application
+forbidden_modules =
+    orb.infrastructure
+    orb.providers
+    orb.monitoring
+allow_indirect_imports = True
+
+
+# ---------------------------------------------------------------------------
+# Contract 3: infrastructure-no-upward
+#
+# The infrastructure layer must not import from the interface/CLI layers.
+# It adapts domain ports to external systems; it must not depend on the
+# presentation/CLI tier.
+#
+# I1 baseline violation has been fixed by Epic E4 / ticket #1358:
+#   I1 — console_adapter inverted: it now owns Rich logic and cli/console.py
+#        delegates downward to the adapter (cli → infrastructure, correct).
+# ---------------------------------------------------------------------------
+[importlinter:contract:infrastructure-no-upward]
+name = Infrastructure layer does not import upward layers
+type = forbidden
+source_modules = orb.infrastructure
+forbidden_modules =
+    orb.interface
+    orb.cli
+allow_indirect_imports = True

docs/root/architecture/machine-discovery-flow.md

@@ -44,13 +44,13 @@ sequenceDiagram
 sequenceDiagram
     participant HF as HostFactory
     participant CLI as CLI/API
-    participant QRY as GetRequestHandler
+    participant QRY as SyncAndGetRequestHandler
     participant DB as Storage
     participant AWS as AWS Provider
     participant HAND as AWS Handler
 
     HF->>CLI: getRequestStatus.sh req-12345
-    CLI->>QRY: GetRequestQuery
+    CLI->>QRY: SyncAndGetRequestQuery
     QRY->>DB: Find request by ID
     DB-->>QRY: Request (with resource_ids + provider_api)
     QRY->>DB: Find machines by request_id

docs/root/developer_guide/adding_a_provider.md

@@ -53,7 +53,7 @@ Complete these steps in order. Each one has a corresponding section in the exten
 
 ### 1. Create the provider package
 
-Create `src/orb/providers/<name>/` following the layout above. The strategy class must extend `BaseProviderStrategy` from `orb.providers.base.strategy.base_provider_strategy`.
+Create `src/orb/providers/<name>/` following the layout above. The strategy class must extend `ProviderStrategy` from `orb.providers.base.strategy` (see `src/orb/providers/base/strategy/provider_strategy.py`). Both the AWS and k8s providers use this base — for example, `AWSProviderStrategy(ProviderStrategy)` in `src/orb/providers/aws/strategy/aws_provider_strategy.py`.
 
 ### 2. Add an enum entry
 
@@ -579,6 +579,6 @@ The AWS provider tests are the reference layout:
 ## Cross-references
 
 - [Clean Architecture](../architecture/clean_architecture.md) — layer boundaries enforced by architecture tests
-- [Strategy Pattern](../patterns/strategy_pattern.md) — how `BaseProviderStrategy` and `ProviderRegistry` work together
+- [Strategy Pattern](../patterns/strategy_pattern.md) — how `ProviderStrategy` and `ProviderRegistry` work together
 - [Ports and Adapters](../patterns/ports_and_adapters.md) — the port/registry decoupling pattern used throughout
 - AWS reference implementation: `src/orb/providers/aws/registration.py`

docs/root/developer_guide/cqrs.md

@@ -245,8 +245,8 @@ Queries represent read operations that don't change system state:
 
 ```python
 @dataclass(frozen=True)
-class GetRequestQuery:
-    """Query to get a specific request."""
+class SyncAndGetRequestQuery:
+    """Query to sync provider state and get a specific request."""
     request_id: str
 
     def validate(self) -> None:
@@ -283,14 +283,14 @@ class GetRequestStatusQuery:
 Query handlers retrieve and format data for read operations:
 
 ```python
-class GetRequestQueryHandler:
-    """Handles request retrieval queries."""
+class SyncAndGetRequestHandler:
+    """Handles sync-and-get request queries."""
 
     def __init__(self, request_read_model: RequestReadModel):
         self._read_model = request_read_model
         self._logger = get_logger(__name__)
 
-    async def handle(self, query: GetRequestQuery) -> Optional[RequestDto]:
+    async def handle(self, query: SyncAndGetRequestQuery) -> Optional[RequestDto]:
         """Handle the get request query."""
         try:
             query.validate()
@@ -478,8 +478,8 @@ def configure_cqrs(container: DIContainer) -> Tuple[CommandBus, QueryBus]:
 
     # Register query handlers
     query_bus.register_handler(
-        GetRequestQuery,
-        GetRequestQueryHandler(request_read_model)
+        SyncAndGetRequestQuery,
+        SyncAndGetRequestHandler(request_read_model)
     )
 
     return command_bus, query_bus
@@ -505,7 +505,7 @@ print(f"Created request: {request_id}")
 
 ```python
 # Get a specific request
-query = GetRequestQuery(request_id="req-123")
+query = SyncAndGetRequestQuery(request_id="req-123")
 request = await query_bus.dispatch(query)
 
 if request:

docs/root/developer_guide/dependency_injection.md

@@ -167,11 +167,11 @@ class MigrateProviderConfigHandler:
 #### Query Handlers (Actual Examples)
 
 ```python
-# From src/application/queries/handlers.py
+# From src/application/queries/request_query_handlers.py
 @injectable
-class GetRequestHandler:
+class SyncAndGetRequestHandler:
     def handle(self, query):
-        # Handle request retrieval
+        # Handle sync-and-get request retrieval
         pass
 
 # From src/application/queries/system_handlers.py
@@ -206,7 +206,7 @@ class UpdateRequestStatusCommand(Command, BaseModel):
     # ... other fields
 
 # From src/application/dto/queries.py
-class GetRequestQuery(Query, BaseModel):
+class SyncAndGetRequestQuery(Query, BaseModel):
     request_id: str
     # ... other fields
 
@@ -286,7 +286,7 @@ Based on actual codebase analysis:
   - `MigrateProviderConfigHandler`
   - And many more...
 - Query handlers in `src/application/queries/`:
-  - `GetRequestHandler`
+  - `SyncAndGetRequestHandler`
   - `GetProviderConfigHandler`
   - `GetActiveMachineCountHandler`
   - And many more...

docs/root/sdk/api-reference.md

@@ -98,7 +98,7 @@ Create a return request for one or more machines.
 
 Retrieve the current status of a provisioning or return request.
 
-**CQRS query:** `GetRequestQuery`
+**CQRS query:** `SyncAndGetRequestQuery`
 
 | Interface | Parameter | CQRS field | Notes |
 |-----------|-----------|------------|-------|
@@ -145,7 +145,7 @@ Note: the REST handler uses `ListRequestsQuery` (from `orb.application.request.q
 
 List requests pending machine return.
 
-**CQRS query:** `ListReturnRequestsQuery`
+**CQRS query:** `SyncAndListReturnRequestsQuery`
 
 | Interface | Parameter | CQRS field | Notes |
 |-----------|-----------|------------|-------|
@@ -397,9 +397,9 @@ The SDK auto-discovers CQRS handlers and derives method names by stripping the `
 | `CreateTemplateCommand` | `create_template` |
 | `UpdateTemplateCommand` | `update_template` |
 | `DeleteTemplateCommand` | `delete_template` |
-| `GetRequestQuery` | `get_request` |
+| `SyncAndGetRequestQuery` | `get_request` |
 | `ListActiveRequestsQuery` | `list_active_requests` |
-| `ListReturnRequestsQuery` | `list_return_requests` |
+| `SyncAndListReturnRequestsQuery` | `list_return_requests` |
 | `GetTemplateQuery` | `get_template` |
 | `ListTemplatesQuery` | `list_templates` |
 | `ValidateTemplateQuery` | `validate_template` |

makefiles/ci.mk

@@ -42,8 +42,12 @@ ci-arch-file-sizes:  ## Check file size compliance
 	@echo "Running file size checks..."
 	./dev-tools/quality/dev_tools_runner.py check-file-sizes --warn-only
 
+ci-arch-lint-imports: dev-install  ## Run import-linter layer-boundary contracts
+	@echo "Running import-linter layer-boundary checks..."
+	$(call run-tool,lint-imports,)
+
 # Composite target
-ci-architecture: ci-arch-cqrs ci-arch-clean ci-arch-imports ci-arch-file-sizes  ## Run all architecture checks
+ci-architecture: ci-arch-cqrs ci-arch-clean ci-arch-imports ci-arch-file-sizes ci-arch-lint-imports  ## Run all architecture checks
 
 # Individual security targets (with tool names)
 ci-security-bandit:  ## Run Bandit security scan

pyproject.toml

@@ -204,6 +204,7 @@ ci = [
     "pyright>=1.1.408,<2.0.0",
     "bandit>=1.7.5,<2.0.0",
     "bandit-sarif-formatter>=1.1.1,<2.0.0",
+    "import-linter>=2.0,<3.0.0",
     # Testing Framework
     "pytest>=7.4.3,<10.0.0",
     "pytest-cov>=4.1.0,<8.0.0",
@@ -293,6 +294,7 @@ ci = [
     "pyright>=1.1.408,<2.0.0",
     "bandit>=1.7.5,<2.0.0",
     "bandit-sarif-formatter>=1.1.1,<2.0.0",
+    "import-linter>=2.0,<3.0.0",  # Architecture layer-boundary enforcement
 
     # Testing Framework
     "pytest>=7.4.3,<10.0.0",

src/orb/api/models/responses.py

@@ -7,7 +7,7 @@
 from pydantic import BaseModel, ConfigDict
 
 from orb.application.request.dto import MachineReferenceDTO
-from orb.infrastructure.error.exception_handler import InfrastructureErrorResponse
+from orb.infrastructure.error.responses import InfrastructureErrorResponse
 
 # ---------------------------------------------------------------------------
 # Pydantic response models (snake_case, default scheduler canonical format)