diff --git a/CHANGELOG.md b/CHANGELOG.md index 77991993c..872cbb5f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,15 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A long message reaches the coworker it is for, and is recorded + +A message over 10,000 characters, such as a pasted email thread or log, was refused by the router +since it started capping the text it reads. The home composer carries on past a routing that fails, +so the message went to the default coworker rather than the one it is for, and a coworker chosen with +`@` or from the To: field started with no `channel.routed` row. The app now asks the router about the +message's opening, and the whole message still goes to the coworker. Shorter messages route as +before. + ### Removing somebody is recorded even when retiring what they owned fails Removing somebody denies their access and ends their sessions, then retires the credentials and diff --git a/app/src/lib/channels/route.ts b/app/src/lib/channels/route.ts index e224a1184..e1f4a3fa0 100644 --- a/app/src/lib/channels/route.ts +++ b/app/src/lib/channels/route.ts @@ -21,13 +21,37 @@ export type RoutingDecision = { viaMention: boolean; }; +/** + * The most of a message `POST /api/route` reads, and a message's opening cut to it. + * + * The route refuses anything longer with a 400, so the prompt it builds stays bounded. The composer + * has no such limit, and both callers here carry on past a routing that failed: the home composer + * sends the message to the default coworker instead, and a chosen coworker's conversation starts + * without its `channel.routed` row. So a pasted email thread or log went to the wrong coworker, or + * unrecorded, and nothing on screen said so. Who a message is for is plain from its opening, so the + * opening is what is asked about; the whole message still goes to the coworker. + * + * Trimmed first, as the route trims, and cut one unit short when the cut would split a character, so + * the router is not handed half of an emoji. + */ +const ROUTING_TEXT_LIMIT = 10_000; + +function routingText(text: string): string { + const trimmed = text.trim(); + if (trimmed.length <= ROUTING_TEXT_LIMIT) return trimmed; + const opening = trimmed.slice(0, ROUTING_TEXT_LIMIT); + const last = opening.charCodeAt(opening.length - 1); + return last >= 0xd800 && last <= 0xdbff ? opening.slice(0, -1) : opening; +} + export async function routeMessage( text: string, agentId?: string, ): Promise { + const asked = routingText(text); const response = await client("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/api/route", { method: "POST", - body: agentId ? { text, agentId } : { text }, + body: agentId ? { text: asked, agentId } : { text: asked }, fallback: "Could not choose a coworker.", }); return (await response.json()) as RoutingDecision; diff --git a/app/tests/route-long-message.test.ts b/app/tests/route-long-message.test.ts new file mode 100644 index 000000000..a1d973723 --- /dev/null +++ b/app/tests/route-long-message.test.ts @@ -0,0 +1,137 @@ +import { afterEach, expect, test } from "bun:test"; +import type { AgentProfileStore } from "../../server/src/agents/profile-store"; +import type { AuditStore } from "../../server/src/audit"; +import type { IntentRouter } from "../../server/src/routing/classify"; +import { createRoutingRoutes } from "../../server/src/routing/routes"; +import { routeMessage } from "../src/lib/channels/route"; + +/** + * A long message still finds its coworker, and the trail still says how. + * + * `POST /api/route` refuses a message over 10,000 characters, so the model prompt it builds stays + * bounded. The composer has no such limit: a pasted email thread or log is one message. Both callers + * of `routeMessage` carry on past a failed routing on purpose, so the refusal said nothing on + * screen. The home composer sent the message to the default coworker instead of the one it is for, + * and a coworker the person chose was started without its `channel.routed` row. + * + * The route itself answers here, not a stub of it, so the cap these hold against is the server's own. + */ + +const originalFetch = globalThis.fetch; +afterEach(() => { + globalThis.fetch = originalFetch; +}); + +const ROSTER = [ + { + id: "general-assistant", + name: "General Assistant", + roleDescription: "everyday work", + visibility: "public", + }, + { + id: "risk-analyst", + name: "Risk Analyst", + roleDescription: "regulatory and compliance questions", + visibility: "public", + }, +]; + +function serve() { + /** What the router was asked to read, so the text it saw is an assertion. */ + const asked: string[] = []; + const written: { eventType: string; payload: Record }[] = []; + + const asActor: Parameters[2] = async ( + context, + next, + ) => { + context.set("actor", { + id: "u1", + email: "person@openbot.test", + role: "user", + }); + await next(); + }; + const store = { list: async () => ROSTER } as unknown as AgentProfileStore; + const router = { + route: async (text: string) => { + asked.push(text); + return { + agentId: "risk-analyst", + name: "Risk Analyst", + reason: "matches what it is for", + fallback: false, + undecided: null, + }; + }, + } as unknown as IntentRouter; + const auditStore = { + insert: async (event: { + eventType: string; + payload: Record; + }) => { + written.push(event); + }, + } as unknown as AuditStore; + + const routes = createRoutingRoutes(store, router, asActor, auditStore); + globalThis.fetch = Object.assign( + async ( + path: Parameters[0], + init?: Parameters[1], + ) => { + if (path !== "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/api/route") throw new Error(`unexpected ${String(path)}`); + return routes.request("http://openbot.test/", init); + }, + { preconnect: originalFetch.preconnect }, + ); + return { asked, written }; +} + +test("a message longer than the route reads is routed to the coworker it is for, and recorded", async () => { + const { asked, written } = serve(); + + const decision = await routeMessage( + `Which of these clauses breach the policy?\n${"clause ".repeat(4_000)}`, + ); + + expect(decision.agentId).toBe("risk-analyst"); + expect(asked).toHaveLength(1); + expect(asked[0]?.startsWith("Which of these clauses")).toBe(true); + expect(written.map((row) => row.eventType)).toEqual(["channel.routed"]); +}); + +test("a long message to a coworker the person chose is recorded as their choice", async () => { + const { asked, written } = serve(); + + const decision = await routeMessage("x".repeat(25_000), "risk-analyst"); + + expect(decision).toMatchObject({ + agentId: "risk-analyst", + viaMention: true, + }); + expect(asked).toEqual([]); + expect(written).toHaveLength(1); + expect(written[0]?.payload).toMatchObject({ + chosen: "risk-analyst", + viaMention: true, + }); +}); + +test("the opening is cut between characters, not through an emoji", async () => { + const { asked } = serve(); + + // The emoji's two halves straddle the 10,000th unit. + await routeMessage(`${"a".repeat(9_999)}😀${"b".repeat(50)}`); + + expect(asked).toEqual(["a".repeat(9_999)]); +}); + +test("a message that fits is sent as it was written", async () => { + const { asked } = serve(); + + await routeMessage("Is this contract compliant? 😀"); + + expect(asked).toEqual(["Is this contract compliant? 😀"]); +});