155 lines
6.8 KiB
Markdown
155 lines
6.8 KiB
Markdown
# 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**
|