From 5669af2b4541054cc890382a27bd363e0dfb8eab Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 19 Sep 2026 02:58:29 +0000 Subject: [PATCH] =?UTF-8?q?fix(agent):=20no=20persistir=20ni=20reenviar=20?= =?UTF-8?q?turnos=20assistant=20vac=C3=ADos?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Un turno solo-tool (p. ej. pauseBot) dejaba role=assistant content="" en D1. Anthropic rechaza bloques de texto vacíos y lastN reinyectaba la fila, así que cada turno siguiente fallaba para siempre. Ahora no se persiste ni se envía un reply vacío, y usableHistory omite filas en blanco al armar el historial para que hilos ya envenenados se recuperen solos. Co-authored-by: santmun --- src/admin/routes.ts | 4 +-- src/agent.ts | 38 +++++++++++++++------ src/db/messages.ts | 10 ++++++ test/admin/suggest.test.ts | 44 +++++++++++++++++++----- test/agent.media.test.ts | 68 ++++++++++++++++++++++++++++++++++++-- test/db/messages.test.ts | 25 +++++++++++++- 6 files changed, 165 insertions(+), 24 deletions(-) diff --git a/src/admin/routes.ts b/src/admin/routes.ts index 05219fb4..3ebff827 100644 --- a/src/admin/routes.ts +++ b/src/admin/routes.ts @@ -52,7 +52,7 @@ import { Db } from "../db/client"; import { LeadsRepo, type Lead } from "../db/leads"; import { TicketsRepo } from "../db/tickets"; import { ConversationsRepo } from "../db/conversations"; -import { MessagesRepo } from "../db/messages"; +import { MessagesRepo, usableHistory } from "../db/messages"; import { SettingsRepo, SETTING_KEYS, type SettingKey } from "../db/settings"; import { CONTROLS, levelToValue } from "./control-levels"; import { systemPromptFromEnv } from "../system-prompt"; @@ -661,7 +661,7 @@ function escapeHtml(s: string): string { // is no per-route auth check here (no magic-link `requireAuth`). adminApp.post("/conversations/:id/suggest", async (c) => { const msgs = new MessagesRepo(new Db(c.env.DB)); - const history = await msgs.lastN(c.req.param("id"), 20); + const history = usableHistory(await msgs.lastN(c.req.param("id"), 20)); const { model } = createModel(c.env, "fast", await loadLlmOverrides(c.env)); const aiMessages = history.map((m) => ({ role: (m.role === "tool" ? "user" : m.role === "owner" ? "assistant" : m.role) as diff --git a/src/agent.ts b/src/agent.ts index 654abc19..5125f9fe 100644 --- a/src/agent.ts +++ b/src/agent.ts @@ -3,7 +3,7 @@ import type { SystemModelMessage } from "ai"; import type { Env } from "./env"; import { Db } from "./db/client"; import { ConversationsRepo } from "./db/conversations"; -import { MessagesRepo } from "./db/messages"; +import { MessagesRepo, usableHistory } from "./db/messages"; import { isPro } from "./config"; import { resolveAgentConfig } from "./settings-loader"; import { buildTools } from "./tools"; @@ -230,8 +230,9 @@ export class SupportAgent extends Agent { await msgs.append(convId, "user", combined); await convs.touchLastMessage(convId); - // Load history (last 20) - const history = await msgs.lastN(convId, 20); + // Load history (last 20). Skip blank rows: a prior tool-only turn may have + // persisted role=assistant content="" and Anthropic rejects empty blocks. + const history = usableHistory(await msgs.lastN(convId, 20)); const aiMessages: any[] = history.slice(0, -1).map((m) => ({ role: (m.role === "tool" ? "user" @@ -418,14 +419,18 @@ export class SupportAgent extends Agent { } } - // Persist assistant message (with usage + model_used + tool calls) - await msgs.append(convId, "assistant", assistantText, { - modelUsed: usedModelId, - inputTokens, - outputTokens, - cachedInputTokens: cachedTokens, - toolCalls: toolCallsMade.length > 0 ? toolCallsMade : undefined, - }); + // Persist only when there is text. A tool-only turn (pauseBot, etc.) is + // valid with empty assistant text; storing "" poisons every later call. + const replyText = assistantText.trim(); + if (replyText) { + await msgs.append(convId, "assistant", assistantText, { + modelUsed: usedModelId, + inputTokens, + outputTokens, + cachedInputTokens: cachedTokens, + toolCalls: toolCallsMade.length > 0 ? toolCallsMade : undefined, + }); + } // Update state for next turn this.setState({ @@ -433,6 +438,17 @@ export class SupportAgent extends Agent { toolCallsInLast2Turns: toolCallCount, }); + // Nothing to say — don't send an empty chunk (Telegram 400s on it). + if (!replyText) { + console.log( + `[SupportAgent.processBuffer] tool-only turn, no reply sent, model=${usedModelId}, cost=$${costOfUsage( + usedModelId, + { input: inputTokens, cached: cachedTokens, output: outputTokens }, + ).toFixed(5)}`, + ); + return; + } + // Chunk + send via the channel adapter const chunks = chunkReply(assistantText, cfg.maxChunks); const channel = this.state.channel as ChannelId; diff --git a/src/db/messages.ts b/src/db/messages.ts index 2413ce3a..ceb92d33 100644 --- a/src/db/messages.ts +++ b/src/db/messages.ts @@ -28,6 +28,16 @@ export interface AppendOptions { createdAt?: number; } +/** Anthropic rejects empty text blocks; whitespace-only is equally poison. */ +export function hasMessageText(content: string | null | undefined): boolean { + return Boolean(content?.trim()); +} + +/** Drop blank rows before sending history to an LLM. */ +export function usableHistory(rows: T[]): T[] { + return rows.filter((m) => hasMessageText(m.content)); +} + export class MessagesRepo { constructor(private readonly db: Db) {} diff --git a/test/admin/suggest.test.ts b/test/admin/suggest.test.ts index f65cf8dc..1c815006 100644 --- a/test/admin/suggest.test.ts +++ b/test/admin/suggest.test.ts @@ -32,14 +32,18 @@ vi.mock("../../src/businessContext", () => ({ // Mock the messages repo so we don't need a real D1 binding. const lastNMock = vi.fn(); -vi.mock("../../src/db/messages", () => ({ - MessagesRepo: class { - constructor(_db: unknown) {} - lastN(id: string, n: number) { - return lastNMock(id, n); - } - }, -})); +vi.mock("../../src/db/messages", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + MessagesRepo: class { + constructor(_db: unknown) {} + lastN(id: string, n: number) { + return lastNMock(id, n); + } + }, + }; +}); import { adminApp } from "../../src/admin/routes"; import type { Env } from "../../src/env"; @@ -114,6 +118,30 @@ describe("admin co-pilot suggestion endpoint", () => { expect(last.content).toContain("asistente del dueño"); }); + it("omite filas vacías del historial antes de llamar al LLM", async () => { + lastNMock.mockResolvedValue([ + { role: "user", content: "Hola" }, + { role: "assistant", content: "" }, + { role: "user", content: "¿siguen ahí?" }, + ]); + + const res = await adminApp.request( + "/conversations/conv-1/suggest", + { method: "POST", headers: { Authorization: basicAuthHeader("admin", PASSWORD) } }, + makeEnv(), + ); + expect(res.status).toBe(200); + + const callArg = generateTextMock.mock.calls[0][0] as { + messages: Array<{ role: string; content: string }>; + }; + expect(callArg.messages.map((m) => m.content)).toEqual([ + "Hola", + "¿siguen ahí?", + expect.stringContaining("asistente del dueño"), + ]); + }); + it("escapes HTML in the LLM output (no injection)", async () => { generateTextMock.mockResolvedValue({ text: "" }); const res = await adminApp.request( diff --git a/test/agent.media.test.ts b/test/agent.media.test.ts index c20cfebb..8a317710 100644 --- a/test/agent.media.test.ts +++ b/test/agent.media.test.ts @@ -62,8 +62,15 @@ vi.mock("@ai-sdk/anthropic", () => ({ })); function makeStreamResult(text: string) { + return makeStreamResultWithTools(text, []); +} + +function makeStreamResultWithTools( + text: string, + toolCalls: { toolName: string; input: unknown }[], +) { async function* gen() { - yield text; + if (text) yield text; } return { textStream: gen(), @@ -72,7 +79,7 @@ function makeStreamResult(text: string) { outputTokens: 50, cachedInputTokens: 0, }), - steps: Promise.resolve([{ toolCalls: [] }]), + steps: Promise.resolve([{ toolCalls }]), }; } @@ -367,6 +374,63 @@ describe("SupportAgent.alarm — multimodal last message (Task 6.3)", () => { expect(sent?.chunks.join("")).toContain("Hola, ¿en qué te ayudo?"); expect(sent?.chunks.join("")).not.toMatch(/Algo falló de mi lado/); }); + + it("tool-only turn: no persiste ni envía un assistant vacío", async () => { + const { agent } = makeAgent({ tier: "free" }); + const sendReply = vi.fn(async () => {}); + const append = vi.fn().mockResolvedValue("msg-id"); + + streamTextMock.mockReset(); + streamTextMock.mockImplementation(() => + makeStreamResultWithTools("", [{ toolName: "pauseBot", input: { minutes: 60 } }]), + ); + generateTextMock.mockReset(); + + vi.spyOn(MessagesRepo.prototype, "append").mockImplementation(append); + vi.spyOn(MessagesRepo.prototype, "lastN").mockResolvedValue([ + { role: "user", content: "hola" }, + ] as any); + vi.spyOn(ConversationsRepo.prototype, "touchLastMessage").mockResolvedValue(undefined as any); + vi.spyOn(senderMod, "pickAdapter").mockReturnValue({ sendReply } as any); + + agent.state.pendingMessages = [{ text: "hola", receivedAt: Date.now() }]; + await agent.processBuffer(); + + const assistantAppends = append.mock.calls.filter((c) => c[1] === "assistant"); + expect(assistantAppends).toHaveLength(0); + expect(append).toHaveBeenCalledWith("conv-1", "user", "hola"); + expect(sendReply).not.toHaveBeenCalled(); + }); + + it("omite filas vacías del historial para que Anthropic no reciba bloques vacíos", async () => { + const { agent } = makeAgent({ tier: "free" }); + + streamTextMock.mockReset(); + streamTextMock.mockImplementation(() => makeStreamResult("ok")); + generateTextMock.mockReset(); + + vi.spyOn(MessagesRepo.prototype, "append").mockResolvedValue(undefined as any); + vi.spyOn(MessagesRepo.prototype, "lastN").mockResolvedValue([ + { role: "user", content: "hola" }, + { role: "assistant", content: "" }, + { role: "assistant", content: " " }, + { role: "user", content: "sigo aquí" }, + ] as any); + vi.spyOn(ConversationsRepo.prototype, "touchLastMessage").mockResolvedValue(undefined as any); + vi.spyOn(senderMod, "pickAdapter").mockReturnValue({ + sendReply: vi.fn(async () => {}), + } as any); + + agent.state.pendingMessages = [{ text: "sigo aquí", receivedAt: Date.now() }]; + await agent.processBuffer(); + + const messages = streamTextMock.mock.calls[0][0].messages as { role: string; content: string }[]; + expect(messages.every((m) => String(m.content ?? "").trim().length > 0)).toBe(true); + expect(messages).toEqual([ + { role: "user", content: "hola" }, + { role: "user", content: "sigo aquí" }, + ]); + }); }); describe("SupportAgent.ingest — bot_paused (settings)", () => { diff --git a/test/db/messages.test.ts b/test/db/messages.test.ts index 1e9173cc..a7e580d0 100644 --- a/test/db/messages.test.ts +++ b/test/db/messages.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach } from "vitest"; import { createTestMiniflare } from "../helpers/miniflareSetup"; import { Db } from "../../src/db/client"; import { ConversationsRepo } from "../../src/db/conversations"; -import { MessagesRepo } from "../../src/db/messages"; +import { MessagesRepo, hasMessageText, usableHistory } from "../../src/db/messages"; let convRepo: ConversationsRepo; let msgRepo: MessagesRepo; @@ -49,3 +49,26 @@ describe("MessagesRepo", () => { expect(msgs[0].content).toBe("new"); }); }); + +describe("usableHistory", () => { + it("drops empty and whitespace-only rows so they never reach the LLM", () => { + const rows = [ + { role: "user", content: "hola" }, + { role: "assistant", content: "" }, + { role: "assistant", content: " " }, + { role: "user", content: "sigo aquí" }, + ]; + expect(usableHistory(rows)).toEqual([ + { role: "user", content: "hola" }, + { role: "user", content: "sigo aquí" }, + ]); + }); + + it("hasMessageText treats null, empty and whitespace as unusable", () => { + expect(hasMessageText("hola")).toBe(true); + expect(hasMessageText("")).toBe(false); + expect(hasMessageText(" \n")).toBe(false); + expect(hasMessageText(null)).toBe(false); + expect(hasMessageText(undefined)).toBe(false); + }); +});