From d482e1a21b338a17d5d1edeee27a129e537db2bd Mon Sep 17 00:00:00 2001 From: Victor Date: Tue, 7 Jul 2026 22:23:41 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20reparar=20sesiones=20corruptas=20(crash?= =?UTF-8?q?=20'no=20tool=20call=20found')=20+=20mejoras=20UX=20de=20conver?= =?UTF-8?q?saci=C3=B3n?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Detectado revisando TODAS las conversaciones reales. P0 — Crash que bloqueaba la sesión: un run cortado a media tool-call dejaba un function_call_output huérfano (sin su call) en el historial, y OpenAI rechazaba CADA turno ('No tool call found for function call output'). El usuario quedaba tirado. Fix: sanitizeItems quita pares de tool incompletos en cualquier posición (no solo al inicio), en cada guardado y al cargar (reparando ficheros ya corruptos). Red de seguridad: si aun así llega el error, ConversationService reinicia la sesión y pide repetir en vez de fallar en silencio. P1 — Donación en punto no elegido: el agente auto-elegía el punto de acopio sin preguntar. Instrucción: nunca elegir el punto por su cuenta; confirmarlo antes de registrar. P2 — Hacer público un recurso: el bot daba vueltas (validar/verificar/publicar) sin tools. Añadidas rg_verify_resource (POST /resources/{id}/verify) y rg_publish_resource (POST /resources/{id}/publish) + instrucción del flujo. Aclarado inventario != necesidades. Copy de botón neutral ('opción con id' en vez de 'centro con ID'). Honestidad de identidad ante 'haz login con +34…' estando ya autenticado. Tests 83->90. --- src/agent/agent.ts | 6 +- src/agent/tools.test.ts | 3 + src/agent/tools.ts | 38 +++++++++ src/application/conversation-service.test.ts | 30 ++++++- src/application/conversation-service.ts | 31 +++++++- .../persistence/file-session-store.ts | 79 +++++++++++++++---- .../persistence/prune-items.test.ts | 48 +++++++++-- 7 files changed, 205 insertions(+), 30 deletions(-) diff --git a/src/agent/agent.ts b/src/agent/agent.ts index 8fddeb4..f256729 100644 --- a/src/agent/agent.ts +++ b/src/agent/agent.ts @@ -18,8 +18,10 @@ Reglas de uso de herramientas: - La emergencia por defecto configurada es: SLUG="${account.emergencySlug}". Las tools usarán esta emergencia por defecto si se omiten los campos. No le preguntes al usuario por la emergencia ni por su slug/ID, asume siempre esta por defecto a menos que el usuario indique explícitamente otra. - Para búsquedas y consultas públicas usa rg_list_public_resources, rg_find_nearby_resources, rg_list_public_needs o rg_find_nearby_needs. CONSULTAR o BUSCAR NUNCA requiere iniciar sesión: no le pidas al usuario que se registre ni que haga login solo para ver qué hay cerca o consultar recursos/necesidades públicas. El login solo hace falta para GESTIONAR/ESCRIBIR (inventario, estado, validar, ofertas autenticadas). Para "cerca de mí" lo único que necesitas es su UBICACIÓN, no su identidad. - Para recursos gestionados usa rg_list_my_managed_resources y luego operaciones de inventario/estado. +- INVENTARIO ≠ NECESIDADES: no los confundas. INVENTARIO es lo que un punto TIENE (rg_get_resource_inventory, rg_record_inventory_entry). NECESIDADES es lo que se PIDE (rg_list_public_needs, rg_list_need_queue, rg_create_need). Si el usuario pide el inventario, muéstrale SOLO el inventario; no le enseñes la cola de necesidades a menos que lo pida. +- HACER PÚBLICO/VISIBLE un recurso: si un recurso gestionado no aparece en el listado público (o te piden su URL pública y no la tiene), el flujo correcto es rg_verify_resource y después rg_publish_resource (requiere ser coordinador/verificador de la emergencia). NO es lo mismo que rg_update_resource_status (eso solo cambia active/saturated/paused/closed de un recurso YA publicado). No des vueltas: si el recurso no es público, propón verificarlo y publicarlo; si el usuario no tiene permiso para ello, díselo con claridad. - Para crear recursos o necesidades, asegúrate de tener los campos mínimos: nombre/título, ubicación con coordenadas, prioridad o tipo, e items cuando aplique. -- DONACIONES (cuando alguien quiere DONAR o LLEVAR material, p. ej. "quiero llevar agua"): NO uses rg_record_inventory_entry — esa es solo para el personal que gestiona el punto y ya recibió stock, y requiere permisos que un donante no tiene. Si la persona quiere entregar material en un punto de acopio, usa rg_preregister_donation (es PÚBLICA, no requiere login): primero ayúdale a elegir el punto (rg_find_nearby_resources / rg_list_public_resources), luego pídele su nombre (y, si quiere, teléfono/email) y registra la donación. Si es un donante ya autenticado que ofrece material de forma general (no a un punto concreto), usa rg_submit_offer con su ubicación real. +- DONACIONES (cuando alguien quiere DONAR o LLEVAR material, p. ej. "quiero llevar agua"): NO uses rg_record_inventory_entry — esa es solo para el personal que gestiona el punto y ya recibió stock, y requiere permisos que un donante no tiene. Si la persona quiere entregar material en un punto de acopio, usa rg_preregister_donation (es PÚBLICA, no requiere login): primero ayúdale a elegir el punto (rg_find_nearby_resources / rg_list_public_resources), luego pídele su nombre (y, si quiere, teléfono/email) y registra la donación. NUNCA elijas el punto de acopio por tu cuenta ni des por hecho el primer resultado: dile cuál propones y espera su confirmación, o pídele que elija; solo registra la donación cuando el usuario haya confirmado explícitamente el punto. Si es un donante ya autenticado que ofrece material de forma general (no a un punto concreto), usa rg_submit_offer con su ubicación real. - Para registrar inventario o crear necesidades con items, es obligatorio seguir este flujo de estandarización y soporte multiidioma: 1. Busca siempre primero los productos solicitados en el catálogo central usando la herramienta rg_search_supplies. Pasa el parámetro locale adecuado (por ejemplo: 'es' si la conversación es en español, 'en' si es en inglés). El parámetro 'q' es de AUTOCOMPLETADO (busca por palabra/prefijo, no de forma semántica): busca con UNA palabra clave concreta —el sustantivo principal—, NO con la frase entera del usuario. Ej.: si pide "comida para bebés", NO busques "comida para bebés" (devuelve 0): busca "bebé", "fórmula", "infantil", "compota" o "cereal". Prueba varios términos y sinónimos. 2. Si encuentras coincidencias, usa su id como supplyId y su nombre correspondiente al idioma de la conversación (nameEs para español, nameEn o name para inglés), sugiriendo estas opciones al usuario para su confirmación. @@ -35,7 +37,7 @@ Seguridad: - Antes de cerrar, pausar, reemplazar inventario completo, validar necesidades o ejecutar cambios sensibles, resume la acción y pide confirmación si no está inequívocamente confirmada. - Si la API responde 401/403, explica que faltan credenciales o permisos, sin inventar la causa exacta. - Si una tool falla con el mensaje "Esta acción requiere que el usuario esté autenticado", NO lo trates como un error técnico: llama a rg_request_user_login para pedirle que inicie sesión y luego reintenta la acción original. -- IDENTIDAD (crítico, no negociable): el inicio de sesión SIEMPRE usa el número de teléfono verificado por la plataforma de mensajería (el número desde el que escribe el usuario). NUNCA puedes iniciar sesión, ni afirmar que lo has hecho, con un teléfono que el usuario te escriba en el texto. Si el usuario te pide "haz login con +34…" o dice que use otro número, explícale con claridad que solo puedes autenticarle con el número verificado de su cuenta de mensajería, y que ignoras cualquier otro número que teclee. No confirmes nunca una sesión con un número distinto al verificado, ni inventes/atribuyas datos (emails, perfiles) a un número que el usuario haya tecleado. Sé honesto: si no puedes hacer algo, dilo; no finjas que lo has hecho. +- IDENTIDAD (crítico, no negociable): el inicio de sesión SIEMPRE usa el número de teléfono verificado por la plataforma de mensajería (el número desde el que escribe el usuario). NUNCA puedes iniciar sesión, ni afirmar que lo has hecho, con un teléfono que el usuario te escriba en el texto. Si el usuario te pide "haz login con +34…" o dice que use otro número, explícale con claridad que solo puedes autenticarle con el número verificado de su cuenta de mensajería, y que ignoras cualquier otro número que teclee. No confirmes nunca una sesión con un número distinto al verificado, ni inventes/atribuyas datos (emails, perfiles) a un número que el usuario haya tecleado. Sé honesto: si no puedes hacer algo, dilo; no finjas que lo has hecho. Si el usuario YA está autenticado (con su número verificado) y te pide "haz login con +34…", NO respondas "ya quedaste autenticado con ese número" (daría a entender que usaste el tecleado): aclárale que ya está identificado con el número verificado de su cuenta de mensajería y que el número que teclea no se utiliza. - Distingue errores de USUARIO de errores TÉCNICOS. Solo pide iniciar sesión (compartir teléfono) cuando el error diga explícitamente que falta autenticación y el usuario aún no se haya identificado. Si el usuario YA está autenticado (p. ej. el login tuvo éxito en este turno) y aun así una tool devuelve 401/403/500 o un error de red, es un problema TÉCNICO nuestro: NO le eches la culpa al usuario, NO le pidas que vuelva a compartir el teléfono ni que reintente en bucle. Discúlpate brevemente, dile que ha habido un problema técnico temporal y que lo intente de nuevo en un momento. EXCEPCIÓN: si el 403 dice explícitamente que falta un PERMISO concreto (p. ej. "Missing permission"), NO es temporal ni un fallo nuestro: significa que esa acción no está permitida para este usuario (a menudo porque elegiste la tool equivocada). No digas "inténtalo de nuevo"; explícalo con honestidad y ofrece la alternativa correcta (p. ej. para donar material usa rg_preregister_donation en vez de registrar inventario). - No muestres al usuario mensajes de error crudos de la API, códigos de estado ni trazas; resume el problema en lenguaje natural. - No muestres tokens, claves ni secretos. diff --git a/src/agent/tools.test.ts b/src/agent/tools.test.ts index 8889853..739cb29 100644 --- a/src/agent/tools.test.ts +++ b/src/agent/tools.test.ts @@ -7,6 +7,9 @@ test("agentTools registra las tools de donación", () => { // Flujo de donante: público (llevar a un punto) y oferta autenticada. assert.ok(names.has("rg_preregister_donation"), "falta rg_preregister_donation"); assert.ok(names.has("rg_submit_offer"), "falta rg_submit_offer"); + // Flujo de hacer público un recurso (verificar + publicar). + assert.ok(names.has("rg_verify_resource"), "falta rg_verify_resource"); + assert.ok(names.has("rg_publish_resource"), "falta rg_publish_resource"); // La tool de inventario sigue existiendo (es de staff, no de donantes). assert.ok(names.has("rg_record_inventory_entry")); }); diff --git a/src/agent/tools.ts b/src/agent/tools.ts index 2a1ff52..c3d13c8 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -494,6 +494,42 @@ export const rgUpdateResourceStatus = tool({ }, }); +export const rgVerifyResource = tool({ + name: "rg_verify_resource", + description: + "Verifica un recurso (paso previo a publicarlo). Requiere ser coordinador/verificador de la emergencia. Úsala cuando un recurso gestionado NO aparece como público y hay que hacerlo visible: primero rg_verify_resource y luego rg_publish_resource.", + parameters: z.object({ + resourceId: z.string().uuid(), + }), + execute: async (input, runContext?: RunContext) => { + const context = getContext(runContext); + requireAuth(context); + const result = await context.apiClient.request( + "POST", + `/resources/${input.resourceId}/verify`, + ); + return asPrettyJson(result); + }, +}); + +export const rgPublishResource = tool({ + name: "rg_publish_resource", + description: + "Publica un recurso para que sea visible públicamente. Requiere ser verificador/coordinador de la emergencia. Es el paso que hace que un recurso gestionado aparezca en el listado público y tenga URL pública (normalmente tras rg_verify_resource).", + parameters: z.object({ + resourceId: z.string().uuid(), + }), + execute: async (input, runContext?: RunContext) => { + const context = getContext(runContext); + requireAuth(context); + const result = await context.apiClient.request( + "POST", + `/resources/${input.resourceId}/publish`, + ); + return asPrettyJson(result); + }, +}); + export const rgListPublicNeeds = tool({ name: "rg_list_public_needs", description: @@ -801,6 +837,8 @@ export const agentTools = [ rgPreregisterDonation, rgSubmitOffer, rgUpdateResourceStatus, + rgVerifyResource, + rgPublishResource, rgListPublicNeeds, rgFindNearbyNeeds, rgCreateNeed, diff --git a/src/application/conversation-service.test.ts b/src/application/conversation-service.test.ts index cf32068..158d4e3 100644 --- a/src/application/conversation-service.test.ts +++ b/src/application/conversation-service.test.ts @@ -1,6 +1,6 @@ import test from "node:test"; import assert from "node:assert"; -import { ConversationService, MAX_TEXT_LENGTH } from "./conversation-service.js"; +import { ConversationService, MAX_TEXT_LENGTH, isCorruptedHistoryError } from "./conversation-service.js"; import { RateLimiter } from "./rate-limiter.js"; import type { Account } from "../domain/account.js"; import type { MessagingChannel, SelectionOption } from "../domain/ports/messaging-channel.port.js"; @@ -164,3 +164,31 @@ test("ConversationService", async (t) => { assert.strictEqual(sent[0].prompt.options[0].id, "inv"); }); }); + +test("isCorruptedHistoryError detecta el error de historial roto", () => { + assert.ok( + isCorruptedHistoryError( + "400 No tool call found for function call output with call_id call_abc.", + ), + ); + assert.ok(!isCorruptedHistoryError("429 rate limited")); +}); + +test("ConversationService recupera de un historial corrupto sin propagar el error", async () => { + const authStore = makeFakeAuthStore(); + let cleared = false; + const session = { ...makeFakeSession(), clearSession: async () => void (cleared = true) } as ConversationStore; + const service = new ConversationService( + { getSession: () => session, authStore, log: () => {} }, + async () => { + throw new Error("400 No tool call found for function call output with call_id call_x."); + }, + ); + const { channel, sent } = makeFakeChannel(); + + await service.handle({ account, chatId: "777", text: "hola" }, channel); // no debe lanzar + + assert.ok(cleared, "reinicia la sesión corrupta"); + assert.strictEqual(sent.length, 1); + assert.match(sent[0].text, /reiniciar/); +}); diff --git a/src/application/conversation-service.ts b/src/application/conversation-service.ts index 056d382..a5b467e 100644 --- a/src/application/conversation-service.ts +++ b/src/application/conversation-service.ts @@ -14,6 +14,19 @@ import type { RateLimiter } from "./rate-limiter.js"; /** Longitud máxima de un mensaje de texto que se procesa (protege coste/abuso). */ export const MAX_TEXT_LENGTH = 8000; +/** + * Detecta el error de OpenAI por historial con un par de tool incompleto + * ("No tool call found for function call output ..."), que bloquea cada turno + * hasta reiniciar la sesión. + */ +export function isCorruptedHistoryError(message: string): boolean { + return ( + /no tool call found/i.test(message) || + /no tool output found/i.test(message) || + /function call output/i.test(message) + ); +} + export interface ConversationServiceDeps { getSession(account: Account, chatId: string): ConversationStore; authStore: AuthStore; @@ -79,12 +92,26 @@ export class ConversationService { try { result = await this.run(apiAgent, userText, { context, session }); } catch (error) { + const message = error instanceof Error ? error.message : String(error); log({ kind: "error", ...logBase, ms: Date.now() - startedAt, - error: error instanceof Error ? error.message : String(error), + error: message, }); + // Red de seguridad: si el historial quedó con un par de tool incompleto, + // OpenAI rechaza cada turno y el usuario queda bloqueado. Reiniciamos la + // sesión y le pedimos que repita, en vez de fallar en silencio. + if (isCorruptedHistoryError(message)) { + await Promise.resolve( + (session as { clearSession?: () => Promise }).clearSession?.(), + ).catch(() => undefined); + await channel.sendText( + chatId, + "Perdona, he tenido que reiniciar nuestra conversación por un problema técnico. ¿Puedes repetir tu último mensaje?", + ); + return; + } throw error; } @@ -109,7 +136,7 @@ export class ConversationService { return `He compartido mi ubicación actual: latitud ${inbound.location.latitude}, longitud ${inbound.location.longitude}`; } if (inbound.selectionCallback) { - return `He seleccionado el centro con ID: ${inbound.selectionCallback}`; + return `He seleccionado la opción con id: ${inbound.selectionCallback}`; } return ""; } diff --git a/src/infrastructure/persistence/file-session-store.ts b/src/infrastructure/persistence/file-session-store.ts index a8d75e2..aac6ed6 100644 --- a/src/infrastructure/persistence/file-session-store.ts +++ b/src/infrastructure/persistence/file-session-store.ts @@ -7,24 +7,15 @@ import { accountKey } from "../../domain/account.js"; /** Máximo de items del historial que se conservan por conversación. */ export const MAX_SESSION_ITEMS = 60; -/** - * Recorta el historial a los últimos `max` items para que el fichero de sesión - * (y el contexto reenviado al LLM) no crezca sin límite. Descarta al principio - * los resultados/salidas de tool "huérfanos" (cuya llamada quedó fuera del - * recorte), que confundirían al modelo; conserva mensajes y llamadas. - */ -export function pruneItems(items: any[], max: number = MAX_SESSION_ITEMS): any[] { - if (!Array.isArray(items) || items.length <= max) { - return items; - } - let trimmed = items.slice(items.length - max); - while (trimmed.length > 0 && isOrphanLeadingResult(trimmed[0])) { - trimmed = trimmed.slice(1); - } - return trimmed; +function callIdOf(item: any): string | undefined { + return item?.callId ?? item?.call_id; } -function isOrphanLeadingResult(item: any): boolean { +function isFunctionCall(item: any): boolean { + return item?.type === "function_call" || item?.type === "tool_call"; +} + +function isFunctionResult(item: any): boolean { const type = item?.type; return ( type === "function_call_result" || @@ -34,6 +25,57 @@ function isOrphanLeadingResult(item: any): boolean { ); } +/** + * Elimina pares de tool incompletos del historial, que hacen que la API de OpenAI + * rechace la petición ("No tool call found for function call output ..."). Ocurre + * cuando un run se corta a media tool-call o el recorte separa la llamada de su + * resultado. SIEMPRE descarta resultados/salidas huérfanas (sin su llamada), en + * cualquier posición. Con `dropDanglingCalls` (solo al cargar, cuando la sesión + * está en reposo) descarta también llamadas sin resultado; en pleno run NO se usa + * porque el resultado puede estar a punto de añadirse. + */ +export function sanitizeItems( + items: any[], + opts: { dropDanglingCalls?: boolean } = {}, +): any[] { + if (!Array.isArray(items)) { + return items; + } + const callIds = new Set(); + const resultIds = new Set(); + for (const item of items) { + const id = callIdOf(item); + if (!id) continue; + if (isFunctionCall(item)) callIds.add(id); + else if (isFunctionResult(item)) resultIds.add(id); + } + return items.filter((item) => { + const id = callIdOf(item); + if (isFunctionResult(item)) { + // Descarta un resultado solo si tiene callId y su llamada no está presente. + // Sin callId no se puede juzgar: se conserva. + return id ? callIds.has(id) : true; + } + if (opts.dropDanglingCalls && isFunctionCall(item)) { + return id ? resultIds.has(id) : true; + } + return true; + }); +} + +/** + * Recorta el historial a los últimos `max` items para que el fichero de sesión + * (y el contexto reenviado al LLM) no crezca sin límite, y sanea los pares de + * tool incompletos (resultados huérfanos) que el recorte pueda dejar. + */ +export function pruneItems(items: any[], max: number = MAX_SESSION_ITEMS): any[] { + if (!Array.isArray(items)) { + return items; + } + const trimmed = items.length > max ? items.slice(items.length - max) : items; + return sanitizeItems(trimmed); +} + export class FileSession extends MemorySession { private filePath: string; @@ -52,7 +94,10 @@ export class FileSession extends MemorySession { const fileContent = readFileSync(this.filePath, "utf8"); const data = JSON.parse(fileContent); if (data && Array.isArray(data.items)) { - (this as any).items = data.items; + // Repara sesiones ya corruptas en disco (pares de tool incompletos): + // al cargar, la sesión está en reposo, así que también quitamos + // llamadas sin resultado, no solo resultados sin llamada. + (this as any).items = sanitizeItems(data.items, { dropDanglingCalls: true }); } } catch (err) { console.error(`Error loading session file for ${options.sessionId}:`, err); diff --git a/src/infrastructure/persistence/prune-items.test.ts b/src/infrastructure/persistence/prune-items.test.ts index 511b49a..a05aa80 100644 --- a/src/infrastructure/persistence/prune-items.test.ts +++ b/src/infrastructure/persistence/prune-items.test.ts @@ -1,10 +1,10 @@ import test from "node:test"; import assert from "node:assert"; -import { pruneItems } from "./file-session-store.js"; +import { pruneItems, sanitizeItems } from "./file-session-store.js"; const msg = (i: number) => ({ type: "message", role: "user", content: `m${i}` }); -const call = (i: number) => ({ type: "function_call", name: `t${i}` }); -const result = (i: number) => ({ type: "function_call_result", name: `t${i}` }); +const call = (i: number) => ({ type: "function_call", name: `t${i}`, callId: `c${i}` }); +const result = (i: number) => ({ type: "function_call_result", name: `t${i}`, callId: `c${i}` }); test("pruneItems", async (t) => { await t.test("no toca un historial por debajo del máximo", () => { @@ -19,18 +19,50 @@ test("pruneItems", async (t) => { assert.strictEqual(pruned[pruned.length - 1].content, "m99", "conserva el más reciente"); }); - await t.test("descarta resultados de tool huérfanos al principio", () => { + await t.test("descarta resultados de tool huérfanos que deja el recorte", () => { // Tras recortar, el primer item sería un function_call_result sin su llamada. const items = [call(0), result(0), msg(1), msg(2)]; - const pruned = pruneItems(items, 3); // últimos 3 = [result(0), msg1, msg2] -> huérfano al frente - assert.notStrictEqual(pruned[0].type, "function_call_result", "no empieza por un resultado huérfano"); + const pruned = pruneItems(items, 3); // últimos 3 = [result(0), msg1, msg2] -> huérfano + assert.ok( + !pruned.some((it) => it.type === "function_call_result"), + "no queda ningún resultado huérfano", + ); assert.strictEqual(pruned[0].type, "message"); }); - await t.test("conserva una function_call al frente (su resultado viene después)", () => { + await t.test("conserva una function_call con su resultado", () => { const items = [msg(0), call(1), result(1), msg(2)]; const pruned = pruneItems(items, 3); // últimos 3 = [call1, result1, msg2] - assert.strictEqual(pruned[0].type, "function_call", "una llamada al frente sí se conserva"); + assert.strictEqual(pruned[0].type, "function_call", "la llamada con su resultado se conserva"); assert.strictEqual(pruned.length, 3); }); }); + +test("sanitizeItems", async (t) => { + await t.test("quita un resultado huérfano en medio del historial (el bug del crash)", () => { + // Un resultado con callId cuya llamada no está: rompe la API de OpenAI. + const items = [msg(0), result(7), msg(1)]; + const clean = sanitizeItems(items); + assert.deepStrictEqual(clean, [msg(0), msg(1)]); + }); + + await t.test("conserva pares completos", () => { + const items = [msg(0), call(1), result(1), msg(2)]; + assert.deepStrictEqual(sanitizeItems(items), items); + }); + + await t.test("solo con dropDanglingCalls quita una llamada sin resultado", () => { + const items = [msg(0), call(3), msg(1)]; + assert.deepStrictEqual(sanitizeItems(items), items, "por defecto NO quita llamadas colgantes"); + assert.deepStrictEqual( + sanitizeItems(items, { dropDanglingCalls: true }), + [msg(0), msg(1)], + "al cargar (reposo) sí las quita", + ); + }); + + await t.test("conserva resultados sin callId (no se pueden juzgar)", () => { + const noId = { type: "function_call_result", name: "x" }; + assert.deepStrictEqual(sanitizeItems([msg(0), noId]), [msg(0), noId]); + }); +});