From 301c521a772e9eea06892d31665cddb0bba70a88 Mon Sep 17 00:00:00 2001 From: borja Date: Sat, 6 Sep 2025 19:17:38 +0200 Subject: [PATCH] =?UTF-8?q?test:=20a=C3=B1ade=20tests=20de=20reintentos;?= =?UTF-8?q?=20docs:=20actualiza=20README/STATUS=20Phase=204?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: aider (openrouter/openai/gpt-5) --- README.md | 20 +++- STATUS.md | 13 ++- tests/unit/services/response-queue.test.ts | 109 +++++++++++++++++++++ 3 files changed, 136 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 17270cb..de17744 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,12 @@ graph TD 14) Seguridad: no enviar al número del bot (CHATBOT_PHONE_NUMBER). 15) Pruebas: unitarias de cola con mocks de fetch. +Actualización Phase 4 — Etapa 1 (Completada): +- Reintentos activados con backoff exponencial + jitter. +- Nuevos campos: `next_attempt_at`, `last_status_code`. +- Selección de pendientes filtrando por `(status='queued' AND next_attempt_at <= now)`. +- Config por entorno: `RQ_MAX_ATTEMPTS`, `RQ_BASE_BACKOFF_MS`, `RQ_MAX_BACKOFF_MS`. + ## Arquitectura de la cola persistente (MVP) - Estados: queued | processing | sent | failed. - Campos actuales por mensaje: id (PK), recipient, message, status, attempts (0), last_error (nullable), metadata (nullable), created_at, updated_at. @@ -80,7 +86,7 @@ Estado: la tabla response_queue ya está creada e incluida en los tests de DB. - Sin lease; en caso de crash podrían quedar mensajes en processing que requerirán recuperación manual en una iteración futura. ## Plan incremental posterior -- Añadir reintentos con backoff exponencial y jitter. +- Añadir reintentos con backoff exponencial y jitter. (Completado) - Garantizar orden por chat (serialización por recipient). - Introducir lease (lease_until) para tolerancia a fallos y recuperación. - Limpieza/retención y métricas/observabilidad. @@ -102,7 +108,7 @@ Estado: la tabla response_queue ya está creada e incluida en los tests de DB. - Persists task and assignments atomically via `TaskService`. - Builds response with assignment list and includes Evolution API “mentioned” JIDs via `ResponseQueue`. - Task persistence service (`src/tasks/service.ts`) with `created_by` and assignment inserts in a transaction; supports DB injection for tests. -- Response queue persistente con workers en background y envío vía Evolution API (`src/services/response-queue.ts`), persistiendo metadata `{ mentioned: [...] }` y enviándola como `mentioned` en el payload. +- Response queue persistente con workers en background y envío vía Evolution API (`src/services/response-queue.ts`), persistiendo metadata `{ mentioned: [...] }` y enviándola como `mentioned` en el payload, con reintentos (backoff exponencial + jitter) y programación por `next_attempt_at`. - Contacts service and friendly names: `ContactsService` resolves display names via webhooks (CONTACTS_UPDATE/CHATS_UPDATE) and Evolution API fallback; used to render names in outgoing texts (falls back to numbers). Skips network calls under NODE_ENV=test for fast and isolated unit tests. - Notification UX: Always send DM acknowledgment to the creator in a single line (format: ✅ Tarea creada: "description"), DM to each assignee (excluding the creator); optional group notification controlled by `NOTIFY_GROUP_ON_CREATE` (default false) with proper mentions. - Environment variable validation (`src/server.ts`, `src/services/webhook-manager.ts`). @@ -177,11 +183,19 @@ bun test - [x] Soportar mensajes de texto extendido y captions de media (además de conversation). ### Phase 4: Fiabilidad de la cola y observabilidad (Media) -- [ ] Añadir reintentos con backoff exponencial y jitter. +- [x] Añadir reintentos con backoff exponencial y jitter. - [ ] Recuperar ítems en estado `processing` tras reinicios (lease o expiración y requeue). - [ ] Métricas y logging mejorado (contadores de enviados/fallidos, tiempos). - [ ] Limpieza/retención de historiales. +### Phase 4 — Desglose y estado +- Etapa 1 — Reintentos con backoff exponencial + jitter (Completada) + - Parámetros: RQ_MAX_ATTEMPTS (6), RQ_BASE_BACKOFF_MS (5000), RQ_MAX_BACKOFF_MS (3600000). + - Lógica: 2xx → sent; 4xx → failed definitivo; 5xx/red → reintento con `next_attempt_at` hasta MAX_ATTEMPTS. +- Etapa 2 — Recuperación de items en `processing` mediante lease/expiración (Pendiente) +- Etapa 3 — Métricas y observabilidad (Pendiente) +- Etapa 4 — Limpieza/retención (Pendiente) + ### Phase 5: Advanced Features (Low Priority) - [ ] Add task reminders system. - [ ] Implement user permissions system. diff --git a/STATUS.md b/STATUS.md index 6506e4a..6e0ceea 100644 --- a/STATUS.md +++ b/STATUS.md @@ -22,6 +22,7 @@ - Persistencia en DB y envío real a Evolution API - Workers en background activos - Soporte de menciones: persistencia en `metadata` y envío como `mentioned` en el payload + - Reintentos con backoff exponencial + jitter (4xx = fallo definitivo; 5xx/red = reintento hasta RQ_MAX_ATTEMPTS con `next_attempt_at`) - **Comandos** - `/tarea nueva` end-to-end: parseo de descripción y última fecha futura, extracción de asignados desde menciones y tokens `@...`, limpieza de la descripción, persistencia de tarea y asignaciones, y respuesta con menciones. - **Contactos y Nombres** @@ -39,9 +40,9 @@ - **Gestión de Tareas** - Eliminación opcional de tareas y mejoras de edición - **Cola de Respuestas** - - Reintentos con backoff y jitter - - Recuperación de ítems en estado `processing` tras caídas - - Métricas/observabilidad y limpieza/retención + - Recuperación de ítems en estado `processing` tras caídas (lease/expiración) + - Métricas/observabilidad + - Limpieza/retención - **Validaciones** - Permisos de usuario (roles) y pertenencia a grupos (si se requiere política estricta) - **Menciones y nombres** @@ -63,6 +64,12 @@ - `src/tasks/service.ts` - `src/server.ts` +## Phase 4 — Desglose y estado +- Etapa 1 — Reintentos con backoff exponencial + jitter: COMPLETADA. +- Etapa 2 — Recuperación de items en `processing` (lease/expiración): PENDIENTE. +- Etapa 3 — Métricas y observabilidad: PENDIENTE. +- Etapa 4 — Limpieza/retención: PENDIENTE. + ## Commit history and status - Latest status: All unit tests passing; Phase 2 completada; ACK to creator always in single-line format; optional group notify disabled by default; ContactsService avoids network calls under tests; basic name resolution via ContactsService integrated. diff --git a/tests/unit/services/response-queue.test.ts b/tests/unit/services/response-queue.test.ts index 48b6d91..3e9395c 100644 --- a/tests/unit/services/response-queue.test.ts +++ b/tests/unit/services/response-queue.test.ts @@ -106,3 +106,112 @@ describe('ResponseQueue (persistent add)', () => { initializeDatabase(testDb); }); }); + +describe('ResponseQueue (retries/backoff)', () => { + function isoNow(): string { + return new Date().toISOString().replace('T', ' ').replace('Z', ''); + } + function isoFuture(ms: number): string { + return new Date(Date.now() + ms).toISOString().replace('T', ' ').replace('Z', ''); + } + + test('claimNextBatch should respect next_attempt_at (only eligible items are claimed)', () => { + const readyAt = isoNow(); + const laterAt = isoFuture(60_000); + + testDb.prepare(` + INSERT INTO response_queue (recipient, message, next_attempt_at) + VALUES ('111', 'ready', ?) + `).run(readyAt); + + testDb.prepare(` + INSERT INTO response_queue (recipient, message, next_attempt_at) + VALUES ('222', 'later', ?) + `).run(laterAt); + + const claimed = (ResponseQueue as any).claimNextBatch(10) as any[]; + expect(Array.isArray(claimed)).toBe(true); + expect(claimed.length).toBe(1); + expect(claimed[0].recipient).toBe('111'); + expect(claimed[0].message).toBe('ready'); + + const rows = testDb.query(`SELECT recipient, status FROM response_queue ORDER BY id`).all() as any[]; + expect(rows[0].status).toBe('processing'); + expect(rows[1].status).toBe('queued'); + }); + + test('markFailed should set failed status, increment attempts and store status code and error (4xx definitive)', () => { + const now = isoNow(); + testDb.prepare(` + INSERT INTO response_queue (recipient, message, next_attempt_at) + VALUES ('333', 'bad request', ?) + `).run(now); + + const claimed = (ResponseQueue as any).claimNextBatch(1) as any[]; + expect(claimed.length).toBe(1); + const item = claimed[0]; + + (ResponseQueue as any).markFailed(item.id, 'bad request', 400, (item.attempts || 0) + 1); + + const row = testDb.query(`SELECT status, attempts, last_status_code, last_error FROM response_queue WHERE id = ?`).get(item.id) as any; + expect(row.status).toBe('failed'); + expect(Number(row.attempts)).toBe(1); + expect(Number(row.last_status_code)).toBe(400); + expect(String(row.last_error)).toContain('bad request'); + }); + + test('requeueWithBackoff should set queued status, increment attempts and schedule next_attempt_at (5xx retry)', () => { + const now = isoNow(); + testDb.prepare(` + INSERT INTO response_queue (recipient, message, next_attempt_at) + VALUES ('444', 'server error', ?) + `).run(now); + + const claimed = (ResponseQueue as any).claimNextBatch(1) as any[]; + expect(claimed.length).toBe(1); + const item = claimed[0]; + const attemptsNow = (item.attempts || 0) + 1; + + const futureWhen = isoFuture(60_000); + (ResponseQueue as any).requeueWithBackoff(item.id, attemptsNow, futureWhen, 500, 'server error'); + + const row = testDb.query(`SELECT status, attempts, next_attempt_at, last_status_code, last_error FROM response_queue WHERE id = ?`).get(item.id) as any; + expect(row.status).toBe('queued'); + expect(Number(row.attempts)).toBe(attemptsNow); + expect(String(row.next_attempt_at)).toBe(futureWhen); + expect(Number(row.last_status_code)).toBe(500); + expect(String(row.last_error)).toContain('server error'); + + // No debería ser reclamable aún + const claimedAgain = (ResponseQueue as any).claimNextBatch(1) as any[]; + expect(claimedAgain.length).toBe(0); + }); + + test('computeDelayMs should return values in [0, max] for several attempts', () => { + const attempts = [1, 2, 3, 4, 5]; + for (const a of attempts) { + const max = Math.min( + (ResponseQueue as any).MAX_BACKOFF_MS, + (ResponseQueue as any).BASE_BACKOFF_MS * (2 ** Math.max(0, a - 1)) + ); + // correr varias veces para validar límites (sin probar distribución) + for (let i = 0; i < 5; i++) { + const d = (ResponseQueue as any).computeDelayMs(a); + expect(typeof d).toBe('number'); + expect(d).toBeGreaterThanOrEqual(0); + expect(d).toBeLessThanOrEqual(max); + } + } + }); + + test('add() should persist next_attempt_at (scheduler field) along with queued items', async () => { + await (ResponseQueue as any).add([ + { recipient: '555', message: 'hola con next' }, + ]); + + const row = testDb.query(`SELECT next_attempt_at, status FROM response_queue ORDER BY id DESC LIMIT 1`).get() as any; + expect(row).toBeTruthy(); + expect(row.status).toBe('queued'); + expect(row.next_attempt_at).toBeTruthy(); // formato texto no vacío + }); +});