Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/admin/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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
Expand Down
38 changes: 27 additions & 11 deletions src/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -230,8 +230,9 @@ export class SupportAgent extends Agent<Env, SupportAgentState> {
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"
Expand Down Expand Up @@ -418,21 +419,36 @@ export class SupportAgent extends Agent<Env, SupportAgentState> {
}
}

// 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({
...this.state,
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;
Expand Down
10 changes: 10 additions & 0 deletions src/db/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T extends { content?: string | null }>(rows: T[]): T[] {
return rows.filter((m) => hasMessageText(m.content));
}

export class MessagesRepo {
constructor(private readonly db: Db) {}

Expand Down
44 changes: 36 additions & 8 deletions test/admin/suggest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof import("../../src/db/messages")>();
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";
Expand Down Expand Up @@ -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: "<script>alert(1)</script>" });
const res = await adminApp.request(
Expand Down
68 changes: 66 additions & 2 deletions test/agent.media.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -72,7 +79,7 @@ function makeStreamResult(text: string) {
outputTokens: 50,
cachedInputTokens: 0,
}),
steps: Promise.resolve([{ toolCalls: [] }]),
steps: Promise.resolve([{ toolCalls }]),
};
}

Expand Down Expand Up @@ -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)", () => {
Expand Down
25 changes: 24 additions & 1 deletion test/db/messages.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
});
});
Loading