diff --git a/docs/adr/ADR-049-cli-communication-pattern.md b/docs/adr/ADR-049-cli-communication-pattern.md new file mode 100644 index 000000000..b20926150 --- /dev/null +++ b/docs/adr/ADR-049-cli-communication-pattern.md @@ -0,0 +1,134 @@ +# ADR-049: CLI Communication Pattern + +**Status**: Accepted +**Date**: 2026-04-16 +**Issues**: #9899, #9859 + +## Context + +The CleverAgents CLI is the primary user-facing interface for the platform. It is implemented in the Presentation layer and communicates with the Application layer to execute commands. + +### Current State: Direct Import Pattern + +The current CLI implementation uses **direct Python imports** to call Application-layer services: + +```python +# Current pattern (Presentation layer) +from cleveragents.application.plan_service import PlanService +from cleveragents.application.project_service import ProjectService + +def plan_execute(plan_id: str): + service = PlanService() + service.execute(plan_id) +``` + +This creates a **reverse dependency**: the Presentation layer directly imports from the Application layer, bypassing the A2A protocol boundary defined in ADR-001 (Layered Architecture) and ADR-047 (A2A Standard Adoption). + +### The A2A Specification + +ADR-047 establishes that **all client-server interaction must flow through A2A**. The A2A protocol is the sole communication contract between the Presentation and Application layers. In server mode, this is already enforced -- CLI commands route through A2A over HTTP. In local mode, the intent is for CLI commands to route through A2A over stdio (via `A2aLocalFacade`). + +### The Problem + +The direct import pattern creates several issues: + +1. **Architectural violation**: The Presentation layer has a compile-time dependency on the Application layer, violating the layered boundary defined in ADR-001. +2. **Import-linter failures**: The `import-linter` tool flags these imports as boundary violations in CI. +3. **Reverse dependency**: If the Application layer changes its internal API, the CLI breaks -- even if the A2A contract is unchanged. +4. **Testing complexity**: CLI tests must mock Application-layer internals rather than the A2A protocol boundary. +5. **Inconsistency**: Server-mode CLI correctly uses A2A; local-mode CLI bypasses it. + +## Decision + +### Immediate: Local CLI Exemption + +The direct import pattern is **permitted under a documented local CLI exemption** until M9 (v3.8.0). This exemption is granted because: + +- The full A2A migration requires significant refactoring across all CLI command handlers. +- The local-mode A2A transport (`A2aLocalFacade` over stdio) is not yet fully implemented. +- The performance impact of routing all local CLI commands through A2A needs measurement. + +**Exemption Rules**: + +1. The exemption applies **only** to the local (non-server) CLI execution path. +2. All server-mode CLI commands **must** route through A2A (already enforced; no exemption granted). +3. The `import-linter` configuration is updated to explicitly allow the exempted imports with a `# cli-exemption: local-only` annotation in the import-linter config file. +4. **No new direct imports** may be added without a corresponding issue tracking the M9 migration. +5. All exempted imports must be documented in `docs/adr/ADR-049-cli-communication-pattern.md` (this document). + +**Import-Linter Configuration**: + +```ini +[importlinter:contract:cli-to-application] +name = CLI must not import Application layer directly +type = forbidden +source_modules = cleveragents.cli +forbidden_modules = cleveragents.application +ignore_imports = + # cli-exemption: local-only -- tracked for M9 migration + cleveragents.cli.commands.plan:cleveragents.application.plan_service + cleveragents.cli.commands.project:cleveragents.application.project_service + cleveragents.cli.commands.actor:cleveragents.application.actor_service + cleveragents.cli.commands.resource:cleveragents.application.resource_service + cleveragents.cli.commands.action:cleveragents.application.action_service + cleveragents.cli.commands.invariant:cleveragents.application.invariant_service + cleveragents.cli.commands.session:cleveragents.application.session_service + cleveragents.cli.commands.config:cleveragents.application.config_service + cleveragents.cli.commands.skill:cleveragents.application.skill_service + cleveragents.cli.commands.tool:cleveragents.application.tool_service + cleveragents.cli.commands.validation:cleveragents.application.validation_service + cleveragents.cli.commands.automation_profile:cleveragents.application.automation_profile_service + cleveragents.cli.commands.lsp:cleveragents.application.lsp_service +``` + +### M9 Migration: Full A2A Adoption + +In M9 (v3.8.0), the CLI will be fully migrated to use `A2aLocalFacade` for all local-mode commands: + +```python +# Target pattern (M9) +from cleveragents.cli.a2a_local_facade import A2aLocalFacade + +def plan_execute(plan_id: str): + facade = A2aLocalFacade() + facade.call("_cleveragents/plan.execute", {"plan_id": plan_id}) +``` + +**Migration Steps**: + +1. Implement `A2aLocalFacade` with full coverage of all `_cleveragents/` extension methods. +2. Measure performance overhead of local A2A routing (target: < 5ms per command). +3. Replace all direct service imports in CLI command handlers with `facade.call()`. +4. Remove all `# cli-exemption: local-only` annotations from import-linter config. +5. Remove the `ignore_imports` block from the import-linter contract. +6. Validate with full CLI integration test suite. +7. Update this ADR status to "Superseded" and reference the M9 implementation PR. + +## Consequences + +### Positive + +- **Architectural consistency**: Local and server-mode CLI use the same protocol boundary. +- **Testability**: CLI tests mock the A2A facade rather than Application-layer internals. +- **Decoupling**: Application-layer internal API changes do not break the CLI as long as the A2A contract is preserved. +- **Import-linter clean**: No boundary violations in CI after M9 migration. + +### Negative + +- **Performance overhead**: Routing through A2A adds latency (estimated < 5ms for local stdio transport). Acceptable for interactive CLI use. +- **Migration effort**: All CLI command handlers must be refactored in M9. +- **Temporary inconsistency**: Until M9, local-mode CLI bypasses the A2A boundary while server-mode enforces it. + +### Neutral + +- The exemption is explicitly documented and tracked, preventing silent accumulation of additional violations. +- The import-linter configuration makes the exemption visible in CI output. + +## References + +- [ADR-001: Layered Architecture](ADR-001-layered-architecture.md) +- [ADR-047: A2A Standard Adoption](ADR-047-acp-standard-adoption.md) +- [ADR-048: Server Application Architecture](ADR-048-server-application-architecture.md) +- [Spec: M7 CLI Communication Pattern Migration](../specification.md#cli-communication-pattern-migration) +- Issue #9899: Invariant reconciliation multi-phase enforcement +- Issue #9859: ACMS thread-safety model documentation diff --git a/docs/specification.md b/docs/specification.md index 9baeff259..c45d4c0a0 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -89,7 +89,7 @@ The following standards are integrated into the architecture: : A persisted choice point in a plan's decision tree, created during Strategize or Execute. Records the question, chosen option, alternatives, confidence score, rationale, context snapshot, and downstream dependencies. Types: `prompt_definition`, `invariant_enforced`, `strategy_choice`, `subplan_spawn`, `subplan_parallel_spawn`, among others. Supports targeted correction with selective subtree recomputation. Invariant - : A natural-language constraint on plan execution scoped to global, project, action, or plan level. The runtime precedence chain is four-tier: ==plan > action > project > global==. Exception: global invariants marked `non_overridable` always win regardless of scope. Reconciled by the Invariant Reconciliation Actor at the start of Strategize; recorded as `invariant_enforced` decisions that propagate to child plans. + : A natural-language constraint on plan execution scoped to global, project, action, or plan level. The runtime precedence chain is four-tier: ==plan > action > project > global==. Exception: global invariants marked `non_overridable` always win regardless of scope. Reconciled by the Invariant Reconciliation Actor at each phase boundary (before Strategize, Execute, and Apply, and after Apply); recorded as `invariant_enforced` decisions that propagate to child plans. Automation Profile : A named set of confidence thresholds (each `0.0`–`1.0`) gating which plan operations proceed automatically versus requiring human approval. `0.0` = always automatic; `1.0` = always manual. Eight built-in profiles (`manual` through `full-auto`). Custom profiles namespaced as `[[server:]namespace/]name`. Each profile composes a **Safety Profile** that controls hard safety constraints (sandbox, checkpoint, unsafe-tool gating, skill restrictions, cost/retry limits). @@ -18393,6 +18393,17 @@ List invariants at a given scope. Use `--effective` with `--plan` to show the fi !!! adr "Architecture Decision" The plan lifecycle, phase transitions, and plan hierarchy are defined in [ADR-006: Plan Lifecycle](adr/ADR-006-plan-lifecycle.md). +!!! note "Multi-Phase Invariant Enforcement" + + Invariant reconciliation is enforced at **every phase boundary**, not just at the start of Strategize. The Invariant Reconciliation Actor runs: + + - **Before Strategize** -- establishes the effective invariant view for strategy generation + - **Before Execute** -- re-validates invariants against the finalized strategy decisions + - **Before Apply** -- confirms invariants still hold before committing sandbox changes + - **After Apply** -- records final invariant state in the decision tree + + Each enforcement pass records `invariant_enforced` decisions that propagate to child plans, ensuring invariant constraints remain consistent throughout the full plan lifecycle. + A **plan** is the fundamental unit of orchestration and traceability. #### Plan Lifecycle Phases @@ -45732,6 +45743,45 @@ When advanced features are unavailable, the system gracefully degrades. The pipe 3. Try `semantic-embedding` (requires vector) -> if unavailable: 4. Fall back to `simple-keyword` (requires only text search / ripgrep) +#### ACMS Thread Safety + +The ACMS context management system is designed for concurrent access by multiple actors and plan phases. Thread safety is enforced at multiple levels using Python's `threading.RLock` (reentrant lock), which allows the same thread to acquire the lock multiple times without deadlocking -- essential for recursive context assembly operations. + +**Concurrency Contract:** + +| Component | Thread Safety Guarantee | +| :-------- | :---------------------- | +| `ContextAssemblyPipeline` | Fully thread-safe; each `assemble()` call acquires a per-pipeline RLock for the duration of the assembly session | +| `HotContextStore` | Thread-safe reads and writes via RLock; concurrent actors receive isolated views | +| `WarmContextStore` | Thread-safe; RLock guards decision index updates and eviction | +| `ColdContextStore` | Thread-safe; RLock guards archive compaction and retrieval | +| `StrategyExecutor` | Strategies execute in parallel (thread pool); results are merged under RLock | +| `FusionCoordinator` | Thread-safe; RLock guards fragment deduplication and ranking | +| `PerActorContextView` | Each actor view is isolated; no cross-actor locking required | + +**RLock Usage Pattern:** + +```python +class ContextAssemblyPipeline: + def __init__(self): + self._lock = threading.RLock() + + def assemble(self, request: ContextRequest) -> AssembledContext: + with self._lock: + # Strategy execution runs in thread pool (releases GIL) + fragments = self._strategy_executor.run_parallel(request) + # Fusion and finalization under lock + fused = self._fusion_coordinator.fuse(fragments) + return self._finalizer.finalize(fused, request) +``` + +**Key Invariants:** + +- No actor can observe a partially-assembled context; assembly is atomic from the caller's perspective. +- Hot context writes from one actor never corrupt another actor's view; per-actor isolation is enforced at the store level. +- Strategy execution (the most expensive phase) runs concurrently in a thread pool; only the merge/finalization step requires the pipeline lock. +- RLock re-entrancy supports recursive plan hierarchies where a parent plan's context assembly triggers child plan context requests within the same thread. + #### ACMS Performance Characteristics ##### Assembly Latency @@ -47321,3 +47371,151 @@ These architectural invariants must be maintained across all milestones: 8. **BDD tests**: All unit-level tests expressed as Behave/Gherkin scenarios. No xUnit-style tests. 9. **File size limit**: No source file exceeds 500 lines. Split into modules if approaching limit. 10. **Atomic commits**: One logical change per commit. No mixed concerns. + + +--- + +## M7: Advanced Concepts and Deferred Features (v3.6.0) + +!!! info "Milestone Overview" + Milestone 7 (v3.6.0) introduces advanced platform capabilities that build on the stable foundation established in M1-M6. These features address deferred complexity, close architectural gaps identified during earlier milestones, and prepare the platform for enterprise-scale deployments. + +### Overview + +v3.6.0 delivers four major capability clusters: + +1. **Advanced Invariant Lifecycle** - multi-phase enforcement, versioned invariant snapshots, and conflict audit trails +2. **ACMS Observability and Tuning** - context assembly telemetry, per-strategy latency budgets, and adaptive tier promotion +3. **Plan Hierarchy Enhancements** - cross-plan invariant propagation, sibling plan coordination, and plan-level resource locking +4. **CLI Communication Pattern Migration** - formal migration path from direct imports to A2A for all CLI commands + +### Module Definitions + +#### Advanced Invariant Lifecycle + +**Motivation**: The M1-M6 implementation enforces invariants at phase boundaries but does not version invariant snapshots or provide audit trails for conflict resolution decisions. Enterprise deployments require full auditability of why a particular invariant view was computed. + +**Specification**: + +- **Invariant Snapshots**: At each phase boundary enforcement pass, the system captures a versioned `InvariantSnapshot` containing: the full effective invariant view, the conflict resolution decisions made by the Invariant Reconciliation Actor, the precedence chain applied (`plan > action > project > global`), and a timestamp. Snapshots are stored in the decision tree as `invariant_snapshot` decision records. + +- **Conflict Audit Trail**: When the Invariant Reconciliation Actor resolves a conflict between two invariants at different scopes, it records an `invariant_conflict_resolved` decision with: the conflicting invariants, the winning invariant and its scope, the losing invariant and its scope, and the actor's rationale. + +- **Invariant Versioning**: Invariants now carry a `version` field (integer, auto-incremented on update). The effective invariant view records the version of each contributing invariant, enabling point-in-time reconstruction of any historical invariant view. + +- **Non-Overridable Propagation**: Global invariants marked `non_overridable` are now explicitly tagged in child plan invariant views, preventing any child plan from silently overriding them even via plan-scope invariants. + +**CLI Extensions**: + +``` +agents invariant history # Show version history +agents invariant snapshot show # Show snapshot for a phase +agents invariant audit # Show full conflict audit trail +``` + +#### ACMS Observability and Tuning + +**Motivation**: Context assembly is the most latency-sensitive operation in the plan lifecycle. Production deployments need per-strategy latency budgets, assembly telemetry, and adaptive tier promotion to maintain SLA compliance. + +**Specification**: + +- **Assembly Telemetry**: Each `assemble()` call emits a structured `ContextAssemblyTrace` containing: total assembly latency, per-strategy latency breakdown, fragment counts per strategy, fusion deduplication ratio, cache hit/miss rates per tier, and token budget utilization. Traces are emitted to the observability subsystem and accessible via `agents diagnostics context`. + +- **Per-Strategy Latency Budgets**: Context strategies can declare a `max_latency_ms` budget. The `StrategyExecutor` enforces this budget via a per-strategy timeout. Strategies that exceed their budget are cancelled and their partial results are discarded (with a warning in the trace). Budget configuration: + +```yaml +context_strategies: + - name: semantic_embedding + max_latency_ms: 500 + - name: graph_navigation + max_latency_ms: 1000 + - name: temporal_archaeology + max_latency_ms: 2000 +``` + +- **Adaptive Tier Promotion**: The ACMS hot tier now supports adaptive promotion: fragments accessed more than `context.hot.promotion_threshold` times within a plan's lifetime are automatically promoted from warm to hot storage. This reduces repeated cold/warm lookups for frequently-accessed context. + +- **Context Budget Alerts**: When context assembly consumes more than `context.budget.alert_threshold` (default: 80%) of the token budget, the system emits a `context_budget_alert` event. Plans can configure alert handlers (log, escalate, or truncate-strategy). + +#### Plan Hierarchy Enhancements + +**Motivation**: Complex multi-plan workflows require coordination primitives beyond simple parent-child spawning. M7 introduces sibling coordination and resource locking. + +**Specification**: + +- **Cross-Plan Invariant Propagation**: Parent plans can now mark specific invariants as `propagate_to_siblings: true`. When a sibling plan is spawned in the same execution group, it inherits these invariants in addition to its own action-level invariants. Sibling-propagated invariants have lower precedence than the sibling's own plan-scope invariants but higher precedence than project-scope invariants. + +- **Sibling Plan Coordination**: Plans executing in `DEPENDENCY_ORDERED` mode can now declare `coordination_signals` - named boolean flags that one sibling sets and another waits on. This enables producer-consumer patterns without requiring a parent plan to mediate. + +```yaml +# In subplan_spawn decision: +coordination_signals: + produces: [schema_migration_complete] + waits_for: [database_ready] +``` + +- **Plan-Level Resource Locking**: Plans can declare `resource_locks` - exclusive or shared locks on specific resources for the duration of a phase. The lock manager prevents conflicting plans from entering the same phase concurrently on the same resource. + +```yaml +resource_locks: + - resource: my-namespace/production-db + mode: exclusive # or: shared + phases: [execute, apply] +``` + +#### CLI Communication Pattern Migration + +**Context**: The current CLI implementation uses direct Python imports to call application-layer services, bypassing the A2A protocol boundary. This creates a reverse dependency (Presentation to Application direct coupling) that violates the layered architecture defined in ADR-001. + +**Decision**: A formal migration plan is established for M9 (v3.8.0). Until then, the direct import pattern is permitted under a documented **local CLI exemption**: + +- The exemption applies only to the local (non-server) CLI execution path. +- All server-mode CLI commands must route through A2A (already enforced). +- The `import-linter` configuration is updated to explicitly allow the exempted imports with a `# cli-exemption: local-only` annotation. +- No new direct imports may be added without a corresponding issue tracking the M9 migration. + +**Migration Path (M9)**: + +1. Introduce `A2aLocalFacade` as the sole interface between CLI and application layer. +2. Replace all direct service imports in CLI command handlers with `facade.call(method, params)`. +3. Remove the `import-linter` exemption rules. +4. Validate with full integration test suite. + +See [ADR-049: CLI Communication Pattern](adr/ADR-049-cli-communication-pattern.md) for the full decision record. + +### Cross-Cutting Concerns + +#### Backward Compatibility + +All M7 features are additive. Existing plans, actions, invariants, and configurations continue to work without modification. New fields (`version`, `propagate_to_siblings`, `resource_locks`, `coordination_signals`) default to values that preserve existing behavior. + +#### Performance Targets + +| Feature | Target | +| :------ | :----- | +| Invariant snapshot capture | < 5ms per phase boundary | +| Context assembly telemetry overhead | < 1% of total assembly latency | +| Adaptive tier promotion decision | < 1ms per fragment access | +| Resource lock acquisition | < 10ms (uncontested) | + +#### Security Considerations + +- Invariant snapshots are stored in the decision tree and subject to the same access controls as decisions. +- Context assembly traces may contain sensitive context fragments; they are stored encrypted at rest when `storage.encrypt_at_rest` is enabled. +- Resource locks are scoped to the plan's namespace; cross-namespace locking requires explicit `allow_cross_namespace_locks: true` configuration. + +### Integration Points + +- **Decision Tree**: Invariant snapshots and conflict audit records are new decision types integrated into the existing decision tree model. +- **ACMS**: Telemetry traces integrate with the existing observability subsystem (ADR-025). +- **Plan Lifecycle**: Resource locking integrates with the existing phase transition machinery. +- **A2A Protocol**: New `_cleveragents/invariant.snapshot`, `_cleveragents/context.trace`, and `_cleveragents/plan.lock` extension methods are added. + +### Milestone Plan + +| Feature | Status | Issue | +| :------ | :----- | :---- | +| Advanced Invariant Lifecycle | Planned | #9899 | +| ACMS Observability and Tuning | Planned | #9859 | +| Plan Hierarchy Enhancements | Planned | - | +| CLI Communication Pattern Migration | Planned | - |