docs(arch-f): ARCH-F Architecture Review — 10 criteria checked, ✅ PASSED, 2 findings (Agent Loop approval gap, contact_id FK redundant), Phase G clearance granted
This commit is contained in:
Agent Zero
@@ -0,0 +1,154 @@
|
|||||||
|
# ARCH-F Review — Architecture Review nach Phase F
|
||||||
|
|
||||||
|
**Datum:** 2026-08-18
|
||||||
|
**Reviewer:** AI Agent
|
||||||
|
**Phase:** F — Agents (41/41 Tasks done)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Review-Kriterien
|
||||||
|
|
||||||
|
### 1. Permission-Lücken ✅
|
||||||
|
|
||||||
|
**Permission Intersection (User ∩ Agent ∩ Skill ∩ Tool):**
|
||||||
|
- `agent_permissions.py`: `resolve_agent_permissions()` korrekt implementiert
|
||||||
|
- `agent_tools.py`: `get_agent_tools()` filtert Tools nach User-Permissions
|
||||||
|
- Skills orchestrieren Tools aber **erweitern niemals** Permissions
|
||||||
|
- System Admins bekommen alle Tools (korrekt)
|
||||||
|
- `check_agent_execute_permission()` prüft RBAC + Entity-Level Access
|
||||||
|
- `filter_visible_agents()` prüft `agents:read` + EntityPermission
|
||||||
|
- Jeder Tool/Service Call re-checkt Permissions (nicht eingefroren)
|
||||||
|
|
||||||
|
**Fazit:** Keine Permission-Lücken gefunden. Die Intersection-Logik ist solide.
|
||||||
|
|
||||||
|
### 2. Task-Modell-Konsistenz ✅
|
||||||
|
|
||||||
|
- `TASK_TYPES` pattern: `^(todo|approval|follow_up|review|goal|milestone|agent_subtask)$` — konsistent in Schema und Model
|
||||||
|
- Polymorphic Fields: `assignee_type/assignee_id`, `entity_type/entity_id`, `creator_type/creator_id` — korrekt
|
||||||
|
- `contact_id` wird aus `entity_id` abgeleitet (nicht in FK-Spalte gespeichert) — vermeidet FK-Constraint-Probleme
|
||||||
|
- Subtasks: `parent_task_id` self-reference korrekt
|
||||||
|
- Dependencies: `depends_on` JSONB array korrekt
|
||||||
|
- Progress Aggregation: `_recompute_progress()` + `_propagate_parent_status()` korrekt
|
||||||
|
- Success Criteria: `_evaluate_success_criteria()` mit `all_done` und `criteria` shapes
|
||||||
|
- `tenant_id` auf allen Queries und Operations
|
||||||
|
|
||||||
|
**Fazit:** Task-Modell ist konsistent. Keine Inkonsistenzen gefunden.
|
||||||
|
|
||||||
|
### 3. Approval-Integration ⚠️
|
||||||
|
|
||||||
|
**Approval API:** Vollständig implementiert (create, list, get, approve, reject, expire)
|
||||||
|
**Approval Model:** `ApprovalRequest` mit tenant_id, entity_type, entity_id, action, status
|
||||||
|
**Workstream:** `post_approval_request()` korrekt implementiert
|
||||||
|
|
||||||
|
**⚠️ Finding: Agent Loop hat keine Human-in-the-Loop Approval-Integration**
|
||||||
|
- `run_react_loop()` hat keinen `require_approval` oder `human_in_loop` Parameter
|
||||||
|
- Agent kann aktuell keine Approval-Requests während eines Loops pausieren/warten
|
||||||
|
- Approval API existiert aber ist nicht in den Agent Loop integriert
|
||||||
|
- **Empfehlung:** In Phase G oder I integrieren — `require_approval` Parameter in `run_react_loop()`, der den Loop pausiert und eine Approval-Request erstellt
|
||||||
|
|
||||||
|
### 4. Workstream-Konsistenz ✅
|
||||||
|
|
||||||
|
- `post_agent_message()`: Erstellt CommMessage in Agent-Channel
|
||||||
|
- `post_agent_step()`: Postet Thought/Action/Observation als CommBlock
|
||||||
|
- `post_agent_result()`: Postet finales Ergebnis
|
||||||
|
- `post_approval_request()`: Postet Approval-Request in Channel
|
||||||
|
- Alle Funktionen verwenden `tenant_id` und `agent_id` konsistent
|
||||||
|
- `_get_or_create_agent_channel()` für dedizierte Agent-Channels
|
||||||
|
|
||||||
|
**Fazit:** Workstream-Patterns sind konsistent.
|
||||||
|
|
||||||
|
### 5. Zirkuläre Abhängigkeiten ✅
|
||||||
|
|
||||||
|
Import-Test erfolgreich:
|
||||||
|
```
|
||||||
|
from app.ai.agent_loop import run_react_loop
|
||||||
|
from app.ai.agent_permissions import resolve_agent_permissions
|
||||||
|
from app.ai.agent_workstream import post_agent_message
|
||||||
|
from app.ai.data_policy import enforce_data_policy
|
||||||
|
from app.ai.oversight import create_decision_record
|
||||||
|
from app.core.approval import create_approval_request
|
||||||
|
→ No circular imports
|
||||||
|
```
|
||||||
|
|
||||||
|
**Fazit:** Keine zirkulären Abhängigkeiten.
|
||||||
|
|
||||||
|
### 6. Data Policy ✅
|
||||||
|
|
||||||
|
- `enforce_data_policy()` prüft SENSITIVE_FIELDS vor LLM-Calls
|
||||||
|
- Provider Compliance: `_filter_by_provider_compliance()` filtert Felder nach Provider-Datenklassen
|
||||||
|
- `get_provider_compliance()` lädt Provider-Konfiguration aus DB
|
||||||
|
- Tenant-spezifische Provider-Konfiguration
|
||||||
|
|
||||||
|
**Fazit:** Data Policy korrekt implementiert.
|
||||||
|
|
||||||
|
### 7. Transparency ✅
|
||||||
|
|
||||||
|
- `mark_as_ai_generated()`: Fügt `ai_generated` flag + `ai_metadata` (model, provider, timestamp) hinzu
|
||||||
|
- `is_ai_participant()`: Erkennt AI-Participant-Types (agent, ai, system_ai)
|
||||||
|
- Wird von Kommunikation-Plugin und Agent Loop verwendet
|
||||||
|
|
||||||
|
**Fazit:** Transparency-Layer korrekt implementiert.
|
||||||
|
|
||||||
|
### 8. Oversight ✅
|
||||||
|
|
||||||
|
- `DecisionRecord` + `DecisionRecordDB`: Audit-Trail für AI-Entscheidungen
|
||||||
|
- `create_decision_record()`: Erstellt Decision Record mit tenant_id, agent_id, tool_name, decision
|
||||||
|
- `DecisionRecordDB` erbt von `Base`, `TenantMixin`, `OwnedMixin`
|
||||||
|
|
||||||
|
**Fazit:** Oversight korrekt implementiert.
|
||||||
|
|
||||||
|
### 9. Agent Memory ✅
|
||||||
|
|
||||||
|
- `store_agent_memory()`, `retrieve_agent_memory()`, `search_agent_memory()`
|
||||||
|
- Alle Operationen verwenden `tenant_id`
|
||||||
|
- Embedding-basierte Suche korrekt
|
||||||
|
|
||||||
|
**Fazit:** Agent Memory korrekt implementiert.
|
||||||
|
|
||||||
|
### 10. Migrations ✅
|
||||||
|
|
||||||
|
- 0122: Agent Definition Phase F Fields (temperature, max_tokens, max_steps, trace_mode, skill_ids, trigger_config, ai_use_case_metadata)
|
||||||
|
- 0123: approval_requests + ai_decision_records Tabellen (tenant_id, UUID PKs, Indizes)
|
||||||
|
- 0124: Unified Task System (polymorphic fields, subtasks, dependencies)
|
||||||
|
- Alle Migrations korrekt verkettet (0122 → 0123 → 0124)
|
||||||
|
- `tenant_id` auf allen neuen Tabellen/Spalten
|
||||||
|
- UUID Primary Keys (keine Integer IDs)
|
||||||
|
|
||||||
|
**Fazit:** Migrations korrekt und konsistent.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Zusammenfassung
|
||||||
|
|
||||||
|
| Kriterium | Status | Findings |
|
||||||
|
|-----------|--------|----------|
|
||||||
|
| Permission-Lücken | ✅ | Keine — Intersection-Logik solide |
|
||||||
|
| Task-Modell-Konsistenz | ✅ | Konsistent — polymorphic fields korrekt |
|
||||||
|
| Approval-Integration | ⚠️ | Agent Loop hat keine Human-in-the-Loop Integration |
|
||||||
|
| Workstream-Konsistenz | ✅ | Konsistente Patterns |
|
||||||
|
| Zirkuläre Abhängigkeiten | ✅ | Keine |
|
||||||
|
| Data Policy | ✅ | SENSITIVE_FIELDS + Provider Compliance |
|
||||||
|
| Transparency | ✅ | AI-Generated Marking korrekt |
|
||||||
|
| Oversight | ✅ | Decision Records mit Audit-Trail |
|
||||||
|
| Agent Memory | ✅ | tenant_id auf allen Operationen |
|
||||||
|
| Migrations | ✅ | Korrekt verkettet, tenant_id, UUID PKs |
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
|
||||||
|
### ⚠️ ARCH-F-1: Agent Loop fehlt Human-in-the-Loop Approval-Integration
|
||||||
|
**Schwere:** Medium (Design-Gap, kein Bug)
|
||||||
|
**Beschreibung:** `run_react_loop()` hat keinen `require_approval` Parameter. Approval API existiert aber ist nicht in den Agent Loop integriert.
|
||||||
|
**Empfehlung:** In Phase G oder I integrieren — `require_approval` Parameter in `run_react_loop()`, der den Loop pausiert und eine Approval-Request erstellt.
|
||||||
|
|
||||||
|
### ℹ️ ARCH-F-2: contact_id FK redundant mit entity_id
|
||||||
|
**Schwere:** Low (bereits im Code behoben)
|
||||||
|
**Beschreibung:** Task Model hat sowohl `contact_id` (FK zu contacts) als auch `entity_type/entity_id` (polymorphic). Code leitet `contact_id` aus `entity_id` ab, aber die FK-Spalte existiert noch in der DB.
|
||||||
|
**Empfehlung:** In einer zukünftigen Migration die `contact_id` FK-Constraint droppen und die Spalte als nullable belassen (oder entfernen).
|
||||||
|
|
||||||
|
## Fazit
|
||||||
|
|
||||||
|
**ARCH-F Review: ✅ BESTANDEN**
|
||||||
|
|
||||||
|
Phase F Architektur ist solide. Keine Permission-Lücken, keine zirkulären Abhängigkeiten, konsistente Patterns. Ein Design-Gap (Agent Loop Approval-Integration) wurde identifiziert und für Phase G/I empfohlen.
|
||||||
|
|
||||||
|
**Freigabe für Phase G: ✅ erteilt**
|
||||||
Reference in New Issue
Block a user