# Архитектурные слабые места Navi

Список проблем, выявленных в ходе аудита 2026-05-16.
Решаем по одной, сверху вниз. После исправления каждого пункта — отмечать галочкой и обновлять этот файл.

---

## 1. God object `navi/core/agent.py` ✅

**Severity:** Critical
**Файл:** `navi/core/agent.py` (1349 → ~410 строк)
**Проблема:** Класс `Agent` одновременно управляет тремя режимами (`run`, `run_stream`, `run_ephemeral`), компрессией контекста (3 уровня fallback), планированием, анти-столлингом, адаптивным репланингом, подсчётом токенов, stall-детекцией, обработкой изображений и под-агентами.
**Почему блокер:** Любое изменение в одной подсистеме требует правки одного файла. Параллельная работа нескольких разработчиков невозможна без конфликтов. Unit-тесты вынуждены инициализировать весь агент даже для проверки одного метода.
**Направление:** Выделить `PlanningOrchestrator`, `ContextCompressor`, `SubAgentRunner`, `AntiStallMonitor` в отдельные сервисы. `Agent` должен остаться только координатором.

**Решение 2026-05-16:**
- `ContextCompressor` → `navi/core/compressor.py`
- `AntiStallMonitor` → `navi/core/anti_stall.py`
- `SubAgentRunner` → `navi/core/subagent_runner.py`
- `AgentTurnContext` / `StreamState` → `navi/core/agent_run_context.py`
- `_iter_stream_guarded` → `navi/core/stream_guard.py`
- `build_tool_list` / `load_user_enabled_tools` → `navi/core/tool_utils.py`
- `run()` — тонкая обёртка вокруг `run_stream()`
- `run_stream()` делегирует `_compression_events_preturn`, `_compression_events_midturn`, `_consume_stream`, `_execute_tools_with_sink`
- `run_ephemeral()` делегирует `SubAgentRunner.run()`

---

## 2. Глобальные ленивые синглтоны в `navi/api/deps.py`

**Severity:** Critical
**Файл:** `navi/api/deps.py` (строки 46–170)
**Проблема:** `_memory_store`, `_registries`, `_mcp_manager`, `_scheduler`, `_kv_store`, `_session_store`, `_workers` — создаются при первом обращении и живут до перезапуска процесса. Нет явного lifecycle management (shutdown pools, close connections).
**Почему блокер:** Невозможно подменить реализацию в тестах без `monkeypatch` на уровне модуля. Горизонтальное масштабирование (несколько процессов) невозможно, потому что состояние привязано к процессу.
**Направление:** Заменить на явный `AppContainer` или `asynccontextmanager`-зависимости FastAPI с `yield`.

---

## 3. WebSocket handler содержит бизнес-логику

**Severity:** High
**Файл:** `navi/api/websocket.py` (443 строки)
**Проблема:** WebSocket handler занимается: heartbeat, replay/reconnect, оркестрацией запуска агента, аутентификацией, валидацией изображений, управлением глобальным состоянием сессий (`_runs`, `_busy_sessions`, `_session_sockets`), созданием `Agent` напрямую.
**Почему блокер:** При добавлении нового транспорта (SSE, gRPC) придётся дублировать всю оркестрацию. Бизнес-логика просочилась в слой сериализации.
**Направление:** Ввести `AgentSessionOrchestrator` между WebSocket и `Agent`. WebSocket должен заниматься только сериализацией/десериализацией.

---

## 4. Mutable global `settings`

**Severity:** High
**Файл:** `navi/config.py`
**Проблема:** `settings = Settings()` доступен из любого модуля. Поля не readonly. `extra="ignore"` означает, что опечатки в `.env` игнорируются без предупреждения.
**Почему блокер:** Любой модуль может изменить `settings.ollama_num_ctx` во время выполнения, что приведёт к race condition при параллельных запросах.
**Направление:** Сделать `Settings` immutable (`frozen=True`). Передавать экземпляр в конструкторы вместо глобального импорта.

**Решение 2026-05-18:**
- `frozen=True` добавлено в `SettingsConfigDict` в `navi/config.py`
- `model_validator(mode="after")` конвертирован в `mode="before"` для загрузки `navi_persona_file`, т.к. frozen-инстанс нельзя мутировать после создания
- Все тесты, которые мутировали поля `settings.field = value`, переведены на замену целого объекта: `monkeypatch.setattr(module, "settings", Settings(...))`
- Для модулей, вызывающих `session_dir` / `ensure_session_dir` из `navi.session_files`, настройки заменяются согласованно в обоих модулях

---

## 5. Дублирование пулов PostgreSQL

**Severity:** High
**Файлы:** `navi/memory/store.py`, `navi/store/__init__.py`, `navi/core/pg_session_store.py`
**Проблема:** Каждый стор создаёт свой `asyncpg.create_pool(self._dsn)` с одинаковым DSN. У каждого своя `asyncio.Lock` для lazy-инициализации.
**Почему блокер:** При росте нагрузки количество соединений к PostgreSQL утраивается. Нет единого менеджера пула.
**Направление:** Вынести `asyncpg.Pool` в отдельный `Database` сервис. Передавать `pool` конструктором в сторы.

**Решение 2026-05-18:**
- Создан `navi/db.py::Database` — единый менеджер пула с lazy-инициализацией
- `KvStore`, `PgSessionStore`, `MemoryStore`, `RecallScheduler` теперь принимают `pool` в `__init__` вместо `dsn`
- `AppContainer` хранит `database: Database`, `shutdown()` закрывает один пул
- `create_container()` создаёт один пул и передаёт его всем сторам
- Lazy-DDL в каждом сторе (через `_initialized` + `_lock`) сохранён для изоляции схем

---

## 6. Кросс-реестровый патчинг в `registry.py`

**Severity:** High
**Файл:** `navi/core/registry.py` (строки 164–254)
**Проблема:** `build_default_registries()` создаёт все реестры, а затем вручную прописывает кросс-ссылки (`list_tool._profile_registry = profiles`, `spawn_tool._backend_registry = backends`).
**Почему блокер:** Циклические зависимости разрешаются через "патч после создания". Добавление нового инструмента требует редактирования фабрики.
**Направление:** Внедрить двухфазную инициализацию: `create()` → `wire()`.

**Решение 2026-05-18:**
- Переупорядочена `build_default_registries`: сначала создаются `backends`, `profiles`, `cp_registry` (нет зависимостей на tools), затем создаются ВСЕ инструменты с полными зависимостями
- Все инструменты получают полные зависимости в конструкторе, без `None` и последующего патчинга:
  `ListToolsTool(registry=tools, profile_registry=profiles, mcp_manager=mcp_manager)`
  `SpawnAgentTool(backend_registry=backends, ...)`
  `ReloadToolsTool(registry=tools, cp_registry=cp_registry, mcp_manager=mcp_manager)`
- Удалены строки патчинга `_profile_registry = profiles`, `_backend_registry = backends`, `_cp_registry = cp_registry`
- Добавлен параметр `mcp_manager` в `build_default_registries` (раньше передавался через патч в `create_container()`)
- Новый инструмент с кросс-зависимостью добавляется в список `builtins` — никакого патчинга не требуется

---

## 7. DRY-нарушение в `tool_executor.py`

**Severity:** Medium
**Файл:** `navi/core/tool_executor.py` (строки 60–187)
**Проблема:** Три метода (`_run_single_tool`, `_execute_tool_calls`, `_execute_tool_calls_streaming`) содержат идентичную логику: resolve → middleware → execute → image extraction → build message.
**Почему блокер:** Любой баг в middleware или image-обработке нужно править в трёх местах.
**Направление:** Единый метод `_execute_one(tc, tool_map) -> (event, msg, image_msg)`, используемый всеми тремя путями.

**Решение 2026-05-18:**
- Введён единый `_execute_one(tc, tool_map)` — единственный канонический путь resolve → middleware → execute → image extraction → build message
- `_run_single_tool`, `_execute_tool_calls`, `_execute_tool_calls_streaming` делегируют `_execute_one`
- Публичные сигнатуры не изменились — никакие вызывающие стороны не пострадали
- Удалено ~77 строк дублирования, добавлено ~21 строк единой логики

---

## 8. Скрытые глобальные зависимости через ContextVar ✅

**Severity:** Medium
**Файл:** `navi/tools/_internal/base.py` (строки 19–42)
**Проблема:** 7 глобальных ContextVar (`current_session_id`, `current_event_sink`, `current_stop_event`, `current_model`, `current_user_id`, `current_user_role`, `current_user_info`). Инструменты читают их неявно.
**Почему блокер:** Инструмент нельзя вызвать вне контекста агента (из CLI, фоновой задачи, теста) без установки всех ContextVar.
**Направление:** Передавать контекст выполнения явным параметром в `execute()`. ContextVar оставить как optional fallback.

**Решение 2026-05-24:**
- Создан `ToolContext` dataclass в `navi/tools/_internal/base.py` — явный контейнер для всех 7 значений
- `Tool.execute()` теперь принимает `ctx: ToolContext | None = None`
- `ToolExecutor._execute_one()` собирает `ToolContext` и передаёт его инструменту
- `Agent._execute_tools_with_sink()` и `SubAgentRunner` строят `ToolContext` из значений в scope и передают в цепочку
- Все ~25 инструментов обновлены: читающие ContextVar теперь предпочитают `ctx`, остальные получили только новый параметр
- Все тесты инструментов переведены на явный `ctx=ToolContext(...)` — больше никаких фикстур с `current_session_id.set()`
- ContextVar setters оставлены как fallback для не-инструментных потребителей (`ai_helper.py`, `context_builder.py`, `planning.py`)

---

## 9. Сессионное состояние в памяти процесса ✅

**Severity:** Medium
**Файл:** `navi/api/websocket.py` (строки 80–86, 403–406)
**Проблема:** `_runs`, `_busy_sessions`, `_session_sockets` — глобальные mutable dict без явной синхронизации. При горизонтальном масштабировании (несколько процессов) состояние запуска не реплицируется.
**Почему блокер:** Сессия может быть запущена на инстансе A, а WebSocket подключён к инстансу B.
**Направление:** Вынести состояние запуска в `SessionStore` (PostgreSQL) или Redis. `_session_sockets` заменить на pub/sub.

**Решение 2026-05-24:**
- Создан `SessionState` dataclass в `navi/core/orchestrator.py` — единый контейнер для `run`, `busy_event`, `websockets`
- `_session_sockets` module-level global удалён из `websocket.py` и перенесён в `AgentSessionOrchestrator._sessions`
- Event bus subscriber `_on_recall_update` перенесён из `websocket.py` в `AgentSessionOrchestrator`
- Добавлен `asyncio.Lock` per session_id — `AgentSessionOrchestrator.session_lock()` защищает concurrent-run guard от race condition
- WebSocket handler теперь использует `orchestrator.add_websocket()` / `remove_websocket()` и `async with orchestrator.session_lock()`
- `_cleanup()` удаляет пустые `SessionState` entries автоматически
- Для горизонтального масштабирования всё ещё требуется Redis/pub-sub (не в scope этого фикса), но in-memory state теперь unified и explicit

---

## 10. MCP: чтение конфига с диска на каждый вызов + retry без backoff

**Severity:** Medium
**Файлы:** `navi/mcp/manager.py`, `navi/mcp/client.py`
**Проблема:** `resolve_group()` и `get_instructions()` вызывают `load_mcp_servers()` при каждом обращении. Нет кэширования. `McpClient` делает reconnect без backoff.
**Почему блокер:** При частых запросах — лишний дисковый I/O. Если MCP-сервер упал, каждый запрос порождает две попытки подключения без задержки.
**Направление:** Закэшировать конфигурацию в `McpManager`. Добавить exponential backoff на reconnect в `McpClient`.

**Решение 2026-05-18:**
- `McpManager` теперь хранит `_configs: dict[str, McpServerConfig]`, заполняемый при `load_all()`
- `resolve_group()` и `get_instructions()` читают из кэша; fallback к диску если кэш пуст (тесты / первый вызов без `load_all()`)
- `reload_all()` сбрасывает кэш (`self._configs = None`) и перечитывает конфиг
- `McpClient` получил exponential backoff: base 1s, max 30s, ±20% jitter
- `_ensure_connected()` блокирует reconnect в пределах backoff-окна; backoff сбрасывается при успешном подключении, удваивается при неудаче

---

## Прогресс

- [x] 1. God object `agent.py` — Step 1 complete: `ContextCompressor` extracted
- [x] 2. Глобальные синглтоны `deps.py` — replaced with AppContainer + lifespan
- [x] 3. WebSocket handler содержит бизнес-логику — extracted AgentSessionOrchestrator
- [x] 4. Mutable global `settings` — frozen Settings + mode="before" validator
- [x] 5. Дублирование пулов PostgreSQL — unified Database service, pool passed to constructors
- [x] 6. Кросс-реестровый патчинг — proper creation order, no post-hoc patching
- [x] 7. DRY-нарушение `tool_executor.py` — unified _execute_one, three methods delegate
- [ ] 8. ContextVar как скрытые зависимости
- [ ] 9. Сессионное состояние в памяти
- [x] 10. MCP кэширование и backoff — cached configs + exponential reconnect backoff