aserrador
30345538f7
- Add `_extract_mcp_content_text()` helper in naliia_tools.py to safely extract text from MCP results, preventing IndexError on empty content - Replace all direct `result.content[0].text` accesses with safe helper - Improve customer identification with preservation of existing state - Add proper JSON parsing with fallback and error handling - Simplify webhook state management (use agent's checkpointer internally) - Update system prompt to remove check_mcp_connection tool reference - Fix tests to use async mocks and correct expected values
140 lines
4.8 KiB
Markdown
140 lines
4.8 KiB
Markdown
# Plan de Corrección: NaliiaTools - Puntos Críticos MCP
|
|
|
|
## Resumen
|
|
|
|
Este documento enumera las tareas de corrección identificadas en `src/naliiabot/bot/tools/naliia_tools.py`, priorizadas por gravedad.
|
|
|
|
---
|
|
|
|
## Tareas por Prioridad
|
|
|
|
### 🔴 PRIORIDAD ALTA
|
|
|
|
#### T-01: `create_schedule` retorna `True` sin validar el resultado del MCP
|
|
**Archivo:** `naliia_tools.py:283-285`
|
|
**Gravedad:** Crítica
|
|
**Descripción:** La función `schedule_appointment` siempre retorna `True` después de hacer la llamada MCP, ignorando completamente el resultado. Si el servidor falla o retorna un error, el agente asume que la cita fue creada exitosamente.
|
|
|
|
**Acción requerida:**
|
|
- [ ] Usar `_extract_mcp_content_text(result)` para extraer el contenido
|
|
- [ ] Validar que el resultado contenga un ID de cita válido
|
|
- [ ] Retornar `False` si la llamada falla o el resultado es inválido
|
|
- [ ] Loguear el contenido real del resultado (no solo `result`)
|
|
|
|
---
|
|
|
|
### 🔴 PRIORIDAD ALTA
|
|
|
|
#### T-02: `asyncio.run()` bloqueante en cada llamada MCP
|
|
**Archivo:** `naliia_tools.py:131, 158, 184, 213, 283`
|
|
**Gravedad:** Alta
|
|
**Descripción:** Cada llamada a herramienta invoca `asyncio.run()`, lo cual bloquea el event loop. En un servidor con múltiples requests, esto causa degradación secuencial.
|
|
|
|
**Acción requerida:**
|
|
- [ ] Opción A: Crear un `async_executor` con `ThreadPoolExecutor` para ejecutar las llamadas async
|
|
- [ ] Opción B: Refactorizar las herramientas para recibir el event loop ya corriendo
|
|
- [ ] Agregar timeouts a las llamadas (`asyncio.wait_for`)
|
|
|
|
---
|
|
|
|
### 🟡 PRIORIDAD MEDIA
|
|
|
|
#### T-03: Sin manejo de reconexión ni retry para `_MCP_CLIENT`
|
|
**Archivo:** `naliia_tools.py:15`
|
|
**Gravedad:** Media
|
|
**Descripción:** El cliente MCP es un singleton global sin retry automático. Si el servidor se cae, todas las herramientas fallan hasta reiniciar.
|
|
|
|
**Acción requerida:**
|
|
- [ ] Crear wrapper con retry exponencial (3 intentos, backoff)
|
|
- [ ] Implementar health check periódico
|
|
- [ ] Agregar timeout global (sugerido: 30s)
|
|
|
|
---
|
|
|
|
### 🟡 PRIORIDAD MEDIA
|
|
|
|
#### T-04: `register_customer` tipo de retorno incorrecto
|
|
**Archivo:** `naliia_tools.py:106`
|
|
**Gravedad:** Media
|
|
**Descripción:** La función declara `-> bool` pero retorna `_extract_mcp_content_text(result)` que es `str`.
|
|
|
|
**Acción requerida:**
|
|
- [ ] Cambiar tipo de retorno a `-> str`
|
|
- [ ] Actualizar docstring para reflejar que retorna el ID del cliente como string
|
|
|
|
---
|
|
|
|
### 🟡 PRIORIDAD MEDIA
|
|
|
|
#### T-05: `check_mcp_connection` no se usa y tiene lógica incorrecta
|
|
**Archivo:** `naliia_tools.py:28-44`
|
|
**Gravedad:** Media
|
|
**Descripción:** La función no está incluida en `get_tools()`. Además, `return True` está hardcodeado dentro de `call_tool()` ignorando el resultado real de `ping()`.
|
|
|
|
**Acción requerida:**
|
|
- [ ] Opción A: Conectar la función al sistema de health checks
|
|
- [ ] Opción B: Eliminar la función si no tiene uso previsto
|
|
- [ ] Si se mantiene, corregir la lógica para retornar el estado real del ping
|
|
|
|
---
|
|
|
|
### 🟢 PRIORIDAD BAJA
|
|
|
|
#### T-06: Sin timeout en llamadas MCP
|
|
**Archivo:** `naliia_tools.py` (todas las llamadas async)
|
|
**Gravedad:** Baja
|
|
**Descripción:** Si el servidor MCP no responde, las llamadas bloquean indefinidamente.
|
|
|
|
**Acción requerida:**
|
|
- [ ] Envolver todas las llamadas con `asyncio.wait_for(coro, timeout=30)`
|
|
- [ ] Capturar `asyncio.TimeoutError` y retornar error apropiado
|
|
|
|
---
|
|
|
|
### 🟢 PRIORIDAD BAJA
|
|
|
|
#### T-07: Mejorar logging de resultados MCP
|
|
**Archivo:** `naliia_tools.py:279`
|
|
**Gravedad:** Baja
|
|
**Descripción:** `logger.info(result)` loguea el objeto completo en lugar del contenido útil.
|
|
|
|
**Acción requerida:**
|
|
- [ ] Cambiar a `logger.info(_extract_mcp_content_text(result))`
|
|
- [ ] Agregar logging de errores con nivel `ERROR`
|
|
|
|
---
|
|
|
|
## Dependencias entre Tareas
|
|
|
|
```
|
|
T-02 (asyncio.run) ──┬── Requerido por T-06 (timeout)
|
|
└── Contexto para T-03 (retry wrapper)
|
|
|
|
T-03 (retry wrapper) ── Requerido por T-05 (health check)
|
|
|
|
T-01 (create_schedule) ── Independiente
|
|
T-04 (register_customer) ── Independiente
|
|
T-07 (logging) ── Independiente
|
|
```
|
|
|
|
---
|
|
|
|
## Orden de Implementación Sugerido
|
|
|
|
1. **T-01** - Fix crítico: `create_schedule` retornando `True` siempre
|
|
2. **T-02** - Mejora de rendimiento: reemplazar `asyncio.run()`
|
|
3. **T-06** - Complemento de T-02: agregar timeouts
|
|
4. **T-03** - Resiliencia: retry y reconexión
|
|
5. **T-04** - Fix de tipos: corregir retorno de `register_customer`
|
|
6. **T-05** - Decisión: conectar o eliminar `check_mcp_connection`
|
|
7. **T-07** - Mejora: logging consistente
|
|
|
|
---
|
|
|
|
## Métricas de Éxito
|
|
|
|
- [ ] Todas las llamadas MCP tienen timeout configurado
|
|
- [ ] `schedule_appointment` retorna `False` cuando falla el MCP
|
|
- [ ] No hay `asyncio.run()` en funciones llamadas desde el agent
|
|
- [ ] Tipos de retorno corresponden a los valores retornados
|