Files
crm-system/architecture-feasibility-review.md
leocrm-bot 3cc0b2e1b4 phase2: architecture, task_graph v2.1.0, AGENTS.md, quality gates, security review
- architecture.md (2019 lines): 73/73 v1 features, RLS policies, CORS, auth rate limiting, v2 FKs removed
- task_graph.json v2.1.0: 14 tasks (7 v1 + 7 v2), 143 features, 298 ACs, all dict test_specs
- AGENTS.md: 14 tasks mapped, T07a/T07b split, v1/v2 phase plan
- Quality gate reviews: Round 1, 2, 3 (all passed)
- Security review: APPROVED_WITH_CONCERNS (0 critical, 7 major, 8 minor)
- Architecture feasibility review: FEASIBLE_WITH_RISKS (3 critical fixed, 5 major fixed)
- All 3 critical issues from feasibility review resolved
- All pre-implementation security items addressed
2026-06-28 23:07:29 +02:00

22 KiB
Raw Permalink Blame History

LeoCRM — Architecture Feasibility Review

Project: leocrm Reviewer: Solution Architect (Agent Zero) Date: 2026-06-28 Documents reviewed: architecture.md (1939 lines), task_graph.json (965 lines, v2.0.0, 13 tasks), AGENTS.md (570 lines), requirements.md (2142 lines, 143 features)


VERDICT: FEASIBLE_WITH_RISKS

The architecture is fundamentally sound — the tech stack is coherent, the multi-tenant design is well-structured, and the API design covers the frontend's needs. However, there are 3 CRITICAL issues in the task graph that must be fixed before implementation can start, plus 5 MAJOR issues that affect feasibility of individual tasks.

Severity Count
CRITICAL 3
MAJOR 5
MINOR 6

1. TASK ORDERING

Finding: Minor inconsistency between execution_plan and parallelization_notes

The dependency graph is correct:

  • T01 → no dependencies (foundation)
  • T02, T03 → depend on T01 only
  • T07, T09, T10 → depend on T01 + T02
  • T04, T05, T06, T11 → depend on T01 + T03
  • T08a, T08b, T08c → depend on T07 + respective backend

No hidden dependencies detected. T02 (Company/Contact) and T03 (Plugin Framework) are genuinely independent after T01. T09 does not depend on T03 (workflow engine uses event bus from T01, not plugin framework). T07 does not depend on T03 (frontend works without plugins — sidebar shows hardcoded v1 items).

Issue: The execution_plan phases don't match the parallelization_notes:

  • T09 is in Phase 3 (alone, parallel: true), T07 is in Phase 4 (parallel: false), T10 is in Phase 5 — but all three depend only on T01+T02 and could run simultaneously.
  • The parallelization_notes correctly states T09 can run parallel with T07, and T10 can run parallel with T07/T09 — but the execution_plan structure implies sequential phases.
  • Recommendation: Merge T09, T07, T10 into a single phase with parallel: true, or add explicit cross-phase parallelism annotations.

Risk level: MINOR — the parallelization_notes clarify intent, but the execution_plan structure could mislead the orchestrator into sequential execution.


2. TASK SIZING

Finding: T07 is critically oversized — MAJOR

T07 (Frontend Core SPA) has:

  • 38 requirement IDs (highest of any task)
  • 32 acceptance criteria (highest of any task)
  • estimated_lines: 600 (wildly underestimated)

Realistic line count breakdown:

Component Estimated Lines
Vite setup + App.tsx + Providers + Router ~150
API Client (axios, interceptors, error handling) ~100
Layout Shell (Sidebar, TopBar, ContentArea) ~200
Auth Pages (Login, Password-Reset Request+Confirm) ~150
Companies Feature (List + TanStack Table + Detail + Tabs + Form) ~350
Contacts Feature (List + Detail + Form) ~300
Settings Feature (Tree + Profile + Role Editor + User Mgmt) ~250
Audit Log Page ~100
Dashboard (Stat Cards + Recent Activity) ~100
Global Search Results Page ~80
i18n Setup (de/en locale files) ~200
Shared UI Component Library (12+ components) ~500
Tailwind CSS + Design Tokens ~50
Total ~2530

At ~2500 lines, T07 is 4× the estimated size and would overwhelm a single implementation block.

Recommendation: Split T07 into:

  • T07a: Frontend Foundation — Vite setup, App.tsx, API Client, Layout Shell, Auth Pages, UI Component Library, i18n, Tailwind, Routing (~1200 lines, ~15 ACs)
  • T07b: Frontend Feature Pages — Companies, Contacts, Settings, Audit Log, Dashboard, Search (~1300 lines, ~17 ACs)
  • T07b depends on T07a

Finding: T09 is borderline — MAJOR

T09 combines two independent subsystems:

  1. KI-Copilot (NL→API translation, LLM client, query/execute/history, RBAC enforcement) — ~350 lines
  2. Workflow Engine (CRUD definitions, instances, step history, code-engine, event triggers, approval timeout) — ~450 lines

Total: ~800 lines, 22 ACs. The two subsystems share no code — only both depend on T01+T02.

Recommendation: Consider splitting into T09a (KI-Copilot) and T09b (Workflow Engine). They can run in parallel. If kept as one task, increase estimated_lines to 800 and ensure the delegation message clearly separates the two modules.


3. TECH STACK FIT

Finding: Stack is coherent — no compatibility concerns

Component Technology Compatibility
Backend framework FastAPI (async Python) Native async, OpenAPI auto-gen
ORM SQLAlchemy 2.0 async + asyncpg Standard for FastAPI
Database PostgreSQL 16 MVCC, tsvector FTS, JSONB, UUID
Cache/Queue Redis 7 Sessions, caching, ARQ queue
Job queue ARQ Async-native, Redis-based, works with FastAPI
Migrations Alembic Standard for SQLAlchemy
Frontend framework React 18 Modern, concurrent features
Build tool Vite Fast HMR, ES modules
Server state TanStack Query v5 Works with React 18, Suspense
Client state Zustand Lightweight, complementary to TanStack Query
Forms React Hook Form + Zod Type-safe validation
Styling Tailwind CSS Utility-first, design tokens
Rich text TipTap React-compatible
PDF viewer PDF.js Mozilla, React-compatible
Document editing OnlyOffice External container, iframe integration
Testing (backend) pytest + httpx Standard for FastAPI
Testing (frontend) Vitest + Testing Library Standard for Vite/React
E2E Playwright Cross-browser, reliable

Minor: Python version not explicitly stated in architecture.md tech stack section. AGENTS.md mentions Python 3.12+ in conventions. Should be in the stack table.

Risk level: NONE — stack is well-established and all components are known-compatible.


4. PLUGIN ARCHITECTURE

Finding: Framework design is sufficient for v2 plugins — but with concerns

T03's plugin framework provides:

  • Plugin Registry (DB-backed)
  • Manifest Schema (Pydantic) with endpoints, migrations, UI, events, services, preferences, notification_types
  • Lifecycle Hooks (install/activate/deactivate/uninstall)
  • Plugin DB Migration Runner with tenant_id validator
  • UI Registry (routes, menu_items, detail_tabs, settings_pages, dashboard_widgets)
  • Event Bus Integration
  • Service Container Injection (db, cache, event_bus, storage, notifications)

v2 plugin requirements vs. framework support:

Plugin Needs Framework Support
DMS routes, events, migrations, UI, storage All provided
Calendar routes, events, migrations, UI, ARQ (reminders) ARQ via container.jobs
Mail routes, events, migrations, UI, IMAP/SMTP ⚠️ IMAP/SMTP NOT in container — plugin implements its own
Tags routes, migrations, UI
Permissions routes, migrations, UI

MAJOR concern: T04 (DMS Backend) and T11 (Tags + Permissions + Entity Links Backend) have identical acceptance criteria for these endpoints:

  • GET /api/v1/dms/files/{id}/permissions
  • POST /api/v1/dms/files/{id}/link
  • DELETE /api/v1/dms/files/{id}/link
  • POST /api/v1/dms/files/{id}/share-link
  • GET /api/public/share/{token} mit expired link → 410
  • DMS plugin listens to company.deleted event → linked files cleanup
  • Folder permissions enforced: user without read → 403

Both tasks are scheduled in Phase 6 (parallel). Two implementers would write the same endpoints → merge conflict.

Additionally, T04's test_spec runs tests/test_tags.py with --cov=app/plugins/builtins/tags — but Tags are T11's responsibility, not T04's.

Recommendation: Clearly separate responsibilities:

  • T04: DMS folders, files, upload, preview, OnlyOffice, bulk operations, DMS search
  • T11: Tags CRUD + assignment, Permissions (file_shares, folder_permissions, share_links), Entity links (file_links)
  • Remove duplicated ACs from one task (keep in the task that owns the endpoint)
  • Remove tests/test_tags.py from T04's test_spec

Risk level: MAJOR — parallel execution with overlapping ACs will cause implementation conflicts.


5. API DESIGN

Finding: Endpoints are sufficient for T07 — one minor gap

T07 frontend needs vs. available endpoints:

Frontend Need API Endpoint Available In
Login/logout/me /api/v1/auth/* T01
Password reset /api/v1/auth/password-reset/* T01
Tenant switch /api/v1/auth/switch-tenant T01
User CRUD /api/v1/users T01
Role CRUD /api/v1/roles T01
User settings /api/v1/users/me/settings T01
Company CRUD + search + export /api/v1/companies T02
Contact CRUD + search + export /api/v1/contacts T02
Company-Contact N:M links /api/v1/companies/{id}/contacts/{cid} T02
Import/preview /api/v1/import, /api/v1/import/preview T02
Audit log /api/v1/audit-log T01
Notifications /api/v1/notifications T01
Global search /api/v1/search T01/T02
Health /api/v1/health T01
Dashboard stats MISSING
Plugin menu items /api/v1/plugins T03 (not a dependency, but done earlier)

MINOR gap: T07's AC says "Dashboard renders with stat cards + recent activity" but there is no GET /api/v1/dashboard or GET /api/v1/stats endpoint. The frontend can compose this from existing endpoints (count companies, count contacts, recent audit log), but an aggregated endpoint would be cleaner.

Recommendation: Either add a GET /api/v1/dashboard endpoint to T01 or T02, or document that the dashboard composes from GET /api/v1/companies?page_size=1 (for total count) + GET /api/v1/contacts?page_size=1 + GET /api/v1/audit-log?page_size=10.

Also: T02 has AC GET /api/v1/companies/{id}/emails → 200 (empty array wenn mail plugin inactive) — this is a v2 endpoint stub in a v1 task. The implementer needs to know to return an empty array when the mail plugin is not active. This should be explicitly documented as a conditional stub.


6. DATABASE SCHEMA

Finding: MAJOR — users table has FK references to v2 tables that don't exist in v1

The users table includes:

default_calendar_id | UUID | FK→calendars.id NULL
default_mail_account_id | UUID | FK→mail_accounts.id NULL

calendars and mail_accounts are v2 plugin tables created by T05 and T06 respectively. In v1, these tables don't exist. The Alembic initial migration (T01) would fail trying to create FK constraints to non-existent tables.

Recommendation:

  1. Remove default_calendar_id and default_mail_account_id from the initial v1 migration
  2. Add these columns in v2 plugin migrations (T05 adds default_calendar_id, T06 adds default_mail_account_id) with FK constraints
  3. Or: add the columns without FK constraints in v1, add FKs in v2 migrations

Other schema findings:

  • plugin_migrations table: described in text (line 1205) but not formally defined as a table with columns. MINOR — should be in the schema section.
  • groups table: referenced by folder_permissions.group_id, file_shares.group_id, calendar_shares.group_id but not defined anywhere. This is a v2 concern but should be documented. MINOR.
  • No user_groups or groups table in the schema — several v2 features reference group-based sharing. MAJOR for v2, but not blocking for v1.
  • All core tables have proper indexes, tenant_id, timestamps, and soft-delete where appropriate.
  • FTS design with tsvector + GIN is correct.
  • UUID PKs with gen_random_uuid() — correct for PostgreSQL 16.

7. FRONTEND ARCHITECTURE

Finding: Detailed enough for T07 implementation — MINOR gaps

The frontend architecture section covers:

  • Full stack table (React 18, Vite, React Router v6, TanStack Query, Zustand, RHF+Zod, Tailwind, etc.)
  • Complete routing table with all routes
  • State management strategy (TanStack Query / Zustand / URL state)
  • i18n setup (de/en, locale files, date-fns)
  • Accessibility (ARIA, 44px, reduced-motion, sr-only, keyboard, WCAG 2.1 AA)
  • Design system based on approved prototype
  • Frontend directory structure

Missing details (MINOR):

  • No API client design (interceptor pattern, error normalization, 401 redirect logic) — mentioned in T07 description but not in architecture.md
  • No data flow diagram (how TanStack Query hooks connect to API client → backend)
  • PluginRegistry.tsx / PluginLoader.tsx described conceptually but no implementation contract
  • No specific file-per-feature breakdown (e.g., what files go in features/companies/)

These gaps are fillable from AGENTS.md conventions and the T07 task description. Not blocking.

Risk level: LOW — architecture + AGENTS.md conventions provide sufficient guidance.


8. KI-COPILOT + WORKFLOW ENGINE

Finding: T09 is under-scoped on requirements but over-scoped on acceptance criteria — MAJOR

T09 has only 5 requirement IDs but 22 acceptance criteria:

  • F-AI-01 → covers entire AI Copilot subsystem (query, execute, history, RBAC, audit, tenant isolation, field permissions) = 8 ACs
  • F-WF-01 → covers entire Workflow Engine (definition CRUD, instances, advance/approve/reject/cancel, event triggers, step history, code-engine, approval timeout) = 14 ACs
  • F-CORE-01 → event bus (shared with T01)
  • F-CORE-06 → API-first (architectural principle, not a feature)
  • F-TEST-01 → testing (shared across all tasks)

The 5 reqs is misleading — F-AI-01 and F-WF-01 are each complex subsystems masquerading as single features. The 22 ACs are well-defined and testable, but implementing two independent subsystems in one delegation is risky.

The two subsystems share no code:

  • KI-Copilot uses: ai_conversations model, LLM client, user session (T01), audit log (T01)
  • Workflow Engine uses: workflows/instances/step_history models, event bus (T01), ARQ (T01)
  • They don't reference each other

Recommendation: Split T09 into:

  • T09a: KI-Copilot API — ai_conversations, LLM client, query/execute/history, RBAC enforcement (~350 lines, 8 ACs)
  • T09b: Workflow Engine — workflow CRUD, instances, code-engine, event triggers, approval timeout (~450 lines, 14 ACs)
  • Both depend on T01+T02, can run in parallel

If kept as one task, increase estimated_lines from 700 to 800+ and ensure the delegation message explicitly separates the two modules with clear file boundaries.


9. DEPLOYMENT READINESS

Finding: Architecture supports Docker/Coolify deployment — MINOR gaps

Present in architecture:

  • Docker Compose with all 6 services (backend, frontend, postgres, redis, worker, onlyoffice)
  • Health checks for backend container
  • Environment variables documented with .env.example plan
  • Backup strategy (pg_dump daily + storage backup)
  • Named volumes for persistent data
  • Non-root container user (app:app) in AGENTS.md forbidden patterns
  • Structured JSON logging for observability
  • Prometheus metrics endpoint planned

Missing (MINOR):

  • No Dockerfiles (backend/frontend) — implementation concern (T01/T07), not architecture
  • No Coolify-specific configuration — Coolify can use docker-compose directly, but no coolify.json or resource limits documented
  • No SSL/TLS documentation — Coolify uses Traefik for LE certificates, but this isn't stated in architecture
  • No resource limits (CPU/memory) for containers — important for multi-tenant production
  • No logging driver configuration for Docker — structured logging is at app level, but Docker log rotation isn't mentioned
  • OnlyOffice container is always in docker-compose — should use Docker Compose profiles for optional services
  • No database initialization script (create DB, run initial migration) documented in deployment flow

Risk level: LOW — all gaps are deployment-phase concerns, not architecture blockers. T10 covers documentation.


10. v1/v2 BOUNDARY

Finding: Boundary is mostly clean — one MAJOR schema issue, one MINOR stub issue

v1 can ship standalone: (with fix)

v1 Task Standalone? Notes
T01 (Core+Auth) Plugin framework ready, no plugins installed
T02 (Company+Contact) Full CRM without plugins
T03 (Plugin Framework) Framework ready, zero plugins activated
T07 (Frontend Core) Companies, contacts, settings, dashboard — no plugin UIs
T09 (KI-Copilot+Workflows) AI + workflows work on core entities
T10 (Monitoring+Docs) Health, metrics, docs for v1 scope

Issues affecting v1 standalone:

MAJOR (blocking): users table FK references to v2 tables (calendars.id, mail_accounts.id) — v1 migration fails. (See §6)

MINOR: T02 has AC GET /api/v1/companies/{id}/emails → 200 (empty array) — this is a v2 mail endpoint stub. The v1 implementer must handle this gracefully (return empty array when mail plugin inactive). Should be documented as a conditional stub, not a full endpoint implementation.

MINOR: T07 company detail shows "Files placeholder, Emails placeholder" tabs — clean v1/v2 boundary via placeholder tabs.

v1/v2 separation in task graph:

  • v1 tasks: T01, T02, T03, T07, T09, T10 (6 tasks)
  • v2 tasks: T04, T05, T06, T11, T08a, T08b, T08c (7 tasks)
  • No v1 task depends on a v2 task
  • No v2 task is required for v1 to function
  • Feature coverage: 73 v1 features / 70 v2 features — all covered

CRITICAL ISSUES (must fix before implementation)

C1: T04/T11 acceptance criteria overlap

Problem: T04 (DMS Backend) and T11 (Tags+Permissions+Links Backend) have 7 identical acceptance criteria for the same endpoints (permissions, file links, share-links, event cleanup, folder permissions). Both are in Phase 6 (parallel) → two implementers writing the same code.

T04's test_spec also includes tests/test_tags.py and --cov=app/plugins/builtins/tags — Tags are T11's responsibility.

Fix:

  • T04 owns: DMS folders, files, upload, preview, OnlyOffice, bulk ops, DMS search
  • T11 owns: Tags CRUD + assignment + bulk, Permissions (file_shares, folder_permissions, share_links), Entity links (file_links), public share endpoint
  • Remove duplicated ACs from T04
  • Remove tests/test_tags.py and tags coverage from T04's test_spec

C2: v2 tasks have non-compliant test_spec

Problem: T08a, T08b, T08c, T11 have test_spec as plain strings instead of structured objects:

"test_spec": "Component tests for file browser, upload, share dialog, tag picker."

Instead of the mandatory structure:

"test_spec": {
  "commands": [...],
  "expected_results": "...",
  "test_files": [...],
  "coverage_target": 80
}

Fix: Convert all 4 tasks' test_spec to structured objects with commands, test_files, expected_results, coverage_target.

C3: v2 tasks use invalid subagent profiles

Problem:

  • T08a, T08b, T08c use "subagent_profile": "frontend_dev" — this profile does not exist
  • T11 uses "subagent_profile": "backend_dev" — this profile does not exist
  • Available profiles: implementation_engineer, developer, etc.

Fix: Change all v2 tasks to "subagent_profile": "implementation_engineer" (matching v1 tasks and AGENTS.md).


MAJOR ISSUES (should fix before implementation)

M1: T07 oversized (38 reqs, 32 ACs, ~2500 lines estimated vs 600 stated)

Split into T07a (Frontend Foundation) + T07b (Frontend Feature Pages).

M2: users table FKs to v2 tables (calendars.id, mail_accounts.id)

Remove from v1 migration, add in v2 plugin migrations.

M3: T09 combines two independent subsystems (KI-Copilot + Workflow Engine, 22 ACs)

Split into T09a (KI-Copilot) + T09b (Workflow Engine), or increase estimated_lines and ensure clear module separation in delegation.

M4: T08a/T08c acceptance criteria are mixed

T08a (DMS+Tags+Permissions) has a Mail AC ("shared mailbox selector"). T08c (Mail+Search) has a DMS AC ("DMS route /dms renders file browser") and a Docker Compose AC. Reassign ACs to correct tasks.

M5: T04 test_spec includes tags tests (T11's responsibility)

Remove tests/test_tags.py and --cov=app/plugins/builtins/tags from T04's test_spec.


MINOR ISSUES (nice to fix, non-blocking)

m1: Execution plan phases don't reflect parallelization notes

T09/T07/T10 are in separate phases but could run parallel. Merge or annotate.

m2: No dashboard/stats endpoint for T07's dashboard stat cards

Add GET /api/v1/dashboard or document client-side composition.

m3: plugin_migrations table not formally defined in schema section

Add table definition with columns.

m4: No Coolify-specific deployment config or SSL/TLS documentation

Document Traefik SSL termination and Coolify deployment flow.

m5: OnlyOffice container always in docker-compose

Use Docker Compose profiles for optional services.

m6: Python version not in tech stack table

Add Python 3.12+ to the stack table in architecture.md §1.


TOP 3 RISKS

  1. T07 oversized task — 38 reqs / 32 ACs / ~2500 lines in one delegation. High probability of incomplete implementation, context window exhaustion, or quality degradation. Must split.

  2. T04/T11 endpoint overlap — 7 identical ACs across two parallel tasks. Will cause merge conflicts, duplicate code, and test failures when both implementers write the same endpoints.

  3. users table FK to non-existent v2 tables — v1 Alembic migration will fail on FK→calendars.id and FK→mail_accounts.id because those tables don't exist until v2 plugin installation.


RECOMMENDATION FOR PHASE 3 START

Conditional GO — fix 3 CRITICAL issues first, then start implementation.

Required actions before implementation:

  1. Fix C1: Separate T04/T11 endpoint ownership, remove duplicate ACs
  2. Fix C2: Convert T08a/T08b/T08c/T11 test_spec to structured objects
  3. Fix C3: Change invalid subagent profiles to implementation_engineer
  4. Fix M1: Split T07 into T07a + T07b
  5. Fix M2: Remove v2 FKs from users table in v1 migration
  6. Fix M4: Reassign mixed ACs between T08a and T08c
  7. Fix M5: Remove tags tests from T04 test_spec

Recommended (non-blocking):

  • Fix M3: Split T09 into T09a + T09b (reduces delegation risk)
  • Fix m1-m6 for documentation quality

After fixes: task_graph.json v2.1.0, architecture.md v1.1, then proceed to implementation.