From efe35a62a22184a42593f5ae37c5f211aa4f46da Mon Sep 17 00:00:00 2001 From: Jerel John Velarde Date: Sun, 30 Aug 2026 07:54:40 -0700 Subject: [PATCH] Build a coworker in one place, and route to one that was named MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four callers now build a Bot for a person — a chat request, a routine's headless turn, a hop delivered to another Bot, and the boundary's own lookup — and each passed the same eleven collaborators positionally. One of them getting an argument wrong is a Bot that runs and quietly holds different tools or a different role from the one the person is talking to. ActorAgentResolver binds them once. Choosing a coworker moves out of the HTTP route for the same reason: it was the routing model call, the visibility rule, and the channel.routed row all written inside a Hono handler, so nothing that is not an HTTP request could route. CoworkerRoutingService owns the decision, and the route turns its outcome into status codes. That move makes an explicit name cheap enough to honour: a message that names exactly one coworker on the asking person's roster no longer pays a model call to be told what the person already said. Two matches are refused with both names rather than guessed at. --- CHANGELOG.md | 16 + server/src/agents/agent-resolver.ts | 101 +++++ server/src/app.ts | 47 ++- server/src/copilot.ts | 119 +----- server/src/index.ts | 221 +++++----- server/src/routing/routes.ts | 211 ++-------- server/src/routing/service.ts | 382 ++++++++++++++++++ server/tests/agent-resolver.test.ts | 112 ++++++ server/tests/copilot.test.ts | 27 +- server/tests/routing-routes.test.ts | 13 +- server/tests/routing-service.test.ts | 576 +++++++++++++++++++++++++++ 11 files changed, 1408 insertions(+), 417 deletions(-) create mode 100644 server/src/agents/agent-resolver.ts create mode 100644 server/src/routing/service.ts create mode 100644 server/tests/agent-resolver.test.ts create mode 100644 server/tests/routing-service.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c851fcd3..032f829a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,22 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A coworker named in the message is routed to without asking a model + +Naming a coworker in the text — "ask Risk Analyst to review this" — went to the intent router like +any other message, so the deployment paid a model call to be told what the person had already said, +and sometimes was told something else. A name that matches exactly one coworker on that person's +roster now routes straight to them, recorded as `named by the person asking` on the same +`channel.routed` row. A name that matches more than one is refused with both names rather than +guessed at, and a name nobody on the roster answers to falls through to the router as before. + +### Routing refuses rather than routes on a connector read it could not make + +Which systems a coworker can reach is weighed by the router alongside what the coworker is for. A +failed read of that used to be treated as "reaches nothing", which is a statement about the +deployment rather than an absence of one: a database that blinked quietly re-routed messages away +from the coworker that could actually do the work. It now fails the request instead. + ### A Bot's shell can no longer reach the embedded database without a password In the all-in-one image the cluster was `trust`-auth on loopback, and the Bot's shell runs in the diff --git a/server/src/agents/agent-resolver.ts b/server/src/agents/agent-resolver.ts new file mode 100644 index 00000000..db7d1e96 --- /dev/null +++ b/server/src/agents/agent-resolver.ts @@ -0,0 +1,101 @@ +import type { AbstractAgent } from "@ag-ui/client"; +import type { AgentFetch, StallGuard } from "../channels/stall-guard"; +import { + type HandoffForRun, + type LoadAgentsForActor, + type LoadToolsForBot, + type RuntimeModel, + resolveRuntimeAgents, + type SignRun, + type ToolSelection, +} from "../copilot"; +import type { AgentActor } from "./profile-types"; + +export type ActorAgentResolver = { + resolveAgentsForActor( + actor: AgentActor, + ): Promise>; + resolveAgentForActor( + actor: AgentActor, + agentId: string, + ): Promise; +}; + +export type ActorAgentResolverDependencies = { + loadAgents: LoadAgentsForActor; + model: RuntimeModel; + resolveModelApiKey: () => Promise; + stallGuard?: StallGuard; + loadToolsForActor?: (actorId: string) => LoadToolsForBot; + signRunForActor?: (actorId: string) => SignRun; + computerGuidance?: string; + loadVendors?: () => Promise; + selectionForActor?: (actorId: string) => ToolSelection; + agentFetch?: AgentFetch; + /** + * What a Bot may reach past itself for, resolved for whoever is asking. + * + * Per actor for the same reason the tools are: which Bots may be reached is decided against the + * roster that person can see, so a Bot must never be able to address one they cannot. + */ + handoffForActor?: (actorId: string) => HandoffForRun; +}; + +/** + * Resolves the coworkers available to one OpenBot actor. + * + * Every surface enters through this boundary so it shares the same visibility, grants, assertions, + * skill selection, and endpoint dial policy for a person. + */ +export function createActorAgentResolver( + deps: ActorAgentResolverDependencies, +): ActorAgentResolver { + const resolveRegisteredAgents = ( + actor: AgentActor, + registered: Awaited>, + /** + * Build only this Bot, when the caller already knows which one it wants. + * + * The roster is still read in full, so a Bot this person cannot see is still absent. The others + * are simply neither built nor asked what they hold, which is a query per Bot a headless turn + * or a Slack thread has no use for. + */ + onlyAgentId?: string, + ) => + resolveRuntimeAgents( + () => Promise.resolve(registered), + deps.model, + deps.resolveModelApiKey, + deps.stallGuard, + deps.loadToolsForActor?.(actor.id), + deps.signRunForActor?.(actor.id), + deps.computerGuidance, + deps.loadVendors, + deps.selectionForActor?.(actor.id), + deps.agentFetch, + deps.handoffForActor?.(actor.id), + onlyAgentId, + ); + + const resolveAgentsForActor = async (actor: AgentActor) => + resolveRegisteredAgents(actor, await deps.loadAgents(actor)); + + return { + resolveAgentsForActor, + async resolveAgentForActor(actor, agentId) { + const registered = await deps.loadAgents(actor); + if (!registered.some((agent) => agent.id === agentId)) { + throw new Error(`Coworker ${agentId} is unavailable to this user.`); + } + + const agents = await resolveRegisteredAgents(actor, registered, agentId); + const agent = Object.hasOwn(agents, agentId) + ? agents[agentId] + : undefined; + if (!agent) { + throw new Error(`Coworker ${agentId} is unavailable to this user.`); + } + return agent; + }, + }; +} diff --git a/server/src/app.ts b/server/src/app.ts index 20561446..e7a5155d 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -44,6 +44,7 @@ import { createRoutineRoutes, type RoutineStore } from "./routines/routes"; import type { RoutineRunner } from "./routines/runner"; import type { IntentRouter } from "./routing/classify"; import { createRoutingRoutes } from "./routing/routes"; +import { createCoworkerRoutingService } from "./routing/service"; import type { PackageStatusReader } from "./tenant-package"; /** @@ -788,29 +789,31 @@ export function createApp( app.route( "/api/route", createRoutingRoutes( - agentProfileStore, - intentRouter, - requireUser, - auditStore, - /* - * Which vendors each coworker holds tools for, so the router weighs what a coworker can - * reach and not only what somebody wrote it was for. Only when there is a plugin store to - * ask: a deployment with no connectors routes exactly as it did. - */ - pluginStore - ? async (agentId) => { - const granted = await pluginStore.listForAgent(agentId); - return [ - ...new Set( - granted.tools.map( - (tool) => - tool.toolName.replace(/^mcp__/, "").split("__")[0] ?? - tool.toolName, + createCoworkerRoutingService({ + store: agentProfileStore, + router: intentRouter, + auditStore, + /* + * Which vendors each coworker holds tools for, so the router weighs what a coworker can + * reach and not only what somebody wrote it was for. Only when there is a plugin store to + * ask: a deployment with no connectors routes exactly as it did. + */ + reachableSystems: pluginStore + ? async (agentId) => { + const granted = await pluginStore.listForAgent(agentId); + return [ + ...new Set( + granted.tools.map( + (tool) => + tool.toolName.replace(/^mcp__/, "").split("__")[0] ?? + tool.toolName, + ), ), - ), - ]; - } - : undefined, + ]; + } + : undefined, + }), + requireUser, ), ); } diff --git a/server/src/copilot.ts b/server/src/copilot.ts index 891c53ee..d8d44f2f 100644 --- a/server/src/copilot.ts +++ b/server/src/copilot.ts @@ -10,10 +10,8 @@ import { createCopilotHonoHandler } from "@copilotkit/runtime/v2/hono"; import type { Observable } from "rxjs"; import { defer, from, switchMap } from "rxjs"; import { z } from "zod"; -import { - COMPUTER_GUIDANCE, - PROVENANCE_GUIDANCE, -} from "../../shared/bot-prompt"; +import { PROVENANCE_GUIDANCE } from "../../shared/bot-prompt"; +import type { ActorAgentResolver } from "./agents/agent-resolver"; import type { AgentActor } from "./agents/profile-types"; import type { AgentFetch, StallGuard } from "./channels/stall-guard"; import type { DeploymentConfig } from "./config"; @@ -856,55 +854,10 @@ export type LoadAgentsForActor = ( */ export function createRequestAgents( identifyActor: IdentifyActor, - loadAgents: LoadAgentsForActor, - model: RuntimeModel, - resolveModelApiKey: () => Promise, - /** - * Shared across every request rather than built per run, because it is the thing that has to - * outlive one: the sweep that notices a silent stream has to still be running after the request - * that opened it has been answered. - */ - stallGuard?: StallGuard, - /** What each Bot may call, resolved for whoever is asking. Absent means no tools. */ - loadToolsForActor?: (actorId: string) => LoadToolsForBot, - /** Resolved per request, because what it signs is who this request turned out to be. */ - signRunForActor?: (actorId: string) => SignRun, - /** What every built-in Bot is told about the computer. Absent means this deployment has none. */ - computerGuidance?: string, - /** Which vendors this deployment connects to, held by a Bot or not. Absent means none. */ - loadVendors?: () => Promise, - /** - * How a run's tools are narrowed, resolved for whoever is asking. - * - * Per actor like the tools themselves, because the skills a Bot holds are read through the same - * grants, and because the discovery row has to name the person the run belongs to. - */ - selectionForActor?: (actorId: string) => ToolSelection, - /** The fetch remote agents are dialled with. See {@link buildAgents}. */ - agentFetch?: AgentFetch, - /** - * How a run gets its tool for handing work to another Bot, resolved for whoever is asking. - * - * Per actor for the same reason the tools are: which Bots may be reached is decided against the - * roster that person can see, so a Bot must never be able to address one they cannot. - */ - handoffForActor?: (actorId: string) => HandoffForRun, + resolver: ActorAgentResolver, ) { return async ({ request }: { request: Request }) => { - const actor = await identifyActor(request); - return resolveRuntimeAgents( - () => loadAgents(actor), - model, - resolveModelApiKey, - stallGuard, - loadToolsForActor?.(actor.id), - signRunForActor?.(actor.id), - computerGuidance, - loadVendors, - selectionForActor?.(actor.id), - agentFetch, - handoffForActor?.(actor.id), - ); + return resolver.resolveAgentsForActor(await identifyActor(request)); }; } @@ -994,26 +947,10 @@ const THREAD_LOCK_TTL_SECONDS = 120; export function mountCopilotRuntime( config: DeploymentConfig, - model: RuntimeModel, - loadAgents: LoadAgentsForActor, - resolveModelApiKey: () => Promise, + resolver: ActorAgentResolver, identifyUser: IdentifyUser, identifyActor: IdentifyActor, - /** - * The watch on Bot streams. Not optional, unlike the parameter it forwards to: a guard built from - * a timeout of zero already watches nothing, so an unconfigured deployment has one to hand and - * there is no reason for a caller to have to say `undefined` here to reach `basePath`. - */ - stallGuard: StallGuard, - loadToolsForActor?: (actorId: string) => LoadToolsForBot, - signRunForActor?: (actorId: string) => SignRun, basePath = "/api/copilotkit", - loadVendors?: () => Promise, - selectionForActor?: (actorId: string) => ToolSelection, - /** The fetch remote agents are dialled with. See {@link buildAgents}. */ - agentFetch?: AgentFetch, - /** How a run gets its tool for handing work on. Absent means no Bot is offered one. */ - handoffForActor?: (actorId: string) => HandoffForRun, ) { const { intelligence } = config.runtime; @@ -1039,25 +976,12 @@ export function mountCopilotRuntime( actor: AgentActor; botId: string; }): Promise => { - const { actor } = input; - const agents = await resolveRuntimeAgents( - () => loadAgents(actor), - model, - resolveModelApiKey, - stallGuard, - loadToolsForActor?.(actor.id), - signRunForActor?.(actor.id), - config.computer ? COMPUTER_GUIDANCE : undefined, - loadVendors, - selectionForActor?.(actor.id), - agentFetch, - handoffForActor?.(actor.id), - // Only the Bot this hop is for. The roster is still read in full, so a Bot this person cannot - // see is still absent; what this skips is constructing the other Bots and asking the database - // what each of them was granted, on every delivery and again on every retry. - input.botId, - ); - return agents[input.botId] ?? null; + // Only the Bot this hop is for. The roster is still read in full, so a Bot this person cannot + // see is still absent; what this skips is constructing the other Bots and asking the database + // what each of them was granted, on every delivery and again on every retry. + return await resolver + .resolveAgentForActor(input.actor, input.botId) + .catch(() => null); }; /* @@ -1088,26 +1012,7 @@ export function mountCopilotRuntime( : {}), // `identifyUser` is the Intelligence projection of the same person `identifyActor` returns: // one resolver decides both whose threads these are and whose coworkers exist. - agents: createRequestAgents( - identifyActor, - loadAgents, - model, - resolveModelApiKey, - stallGuard, - loadToolsForActor, - signRunForActor, - /* - * Only when a computer exists. The tools themselves are registered by the surface, so a Bot is - * offered them without this and the guidance is what tells it how they go together: snapshot - * before acting, and ask a person to take the wheel at a sign-in rather than reporting the task - * as impossible. Absent computer, absent guidance: a Bot is not told about hands it has not got. - */ - config.computer ? COMPUTER_GUIDANCE : undefined, - loadVendors, - selectionForActor, - agentFetch, - handoffForActor, - ) as never, + agents: createRequestAgents(identifyActor, resolver) as never, }); return { diff --git a/server/src/index.ts b/server/src/index.ts index a3b18ef6..f41b3feb 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -5,6 +5,7 @@ import { } from "@copilotkit/runtime/v2"; import { serve } from "bun"; import { COMPUTER_GUIDANCE } from "../../shared/bot-prompt"; +import { createActorAgentResolver } from "./agents/agent-resolver"; import { mintRunAssertion, readRunAssertion } from "./agents/callback-token"; import { createAgentFetch } from "./agents/endpoint"; import { askTheirOwnPerson, escalationTool } from "./agents/escalation"; @@ -47,10 +48,10 @@ import { import { createSnapshotStore } from "./computer/snapshot-store"; import { loadConfig } from "./config"; import { + type HandoffForRun, type IdentifyActor, type IdentifyUser, mountCopilotRuntime, - resolveRuntimeAgents, type ToolSelection, } from "./copilot"; import { @@ -595,6 +596,114 @@ const agentFetch = createAgentFetch({ }, }); +/* + * What a Bot may reach past itself for: another Bot, and a person. Made per run and per person. + * + * Per person because which Bots may be reached is decided against the roster that person can + * see: a Bot must never be able to address one they cannot, or this becomes a way around agent + * visibility. Per run because the caps need to know how deep the chain already is and where an + * answer belongs, and both of those are the deployment's own statement about the run rather than + * anything the model can edit. + */ +const handoffForActor = + (actorId: string): HandoffForRun => + async (botId, input) => { + const from = readRunAssertion( + (input.forwardedProps as { openbotRun?: unknown } | undefined) + ?.openbotRun, + config.keyEncryptionKey, + ); + const run = { + botId, + actorId, + runId: input.runId, + threadId: input.threadId, + depth: from?.depth ?? 0, + }; + /* + * The caps are checked BEFORE the grants query, not inside the tool that would discard it. + * + * `handoffTool` short-circuits on all three of these, but only after being handed a + * `hasSomebodyToAsk` that costs a query. So a deployment which switched the capability off + * still paid one grants read per run of every Bot, for a tool it was never going to be offered, + * and a run already at the cap paid it again. + */ + const couldHandOn = + config.handoff.maxDepth > 0 && + config.handoff.maxPerRun > 0 && + run.depth < config.handoff.maxDepth; + + const passing = couldHandOn + ? handoffTool({ + desk: handoffDesk, + /* + * How deep this run already is comes from the assertion the deployment signed when it handed + * this work on. A run a person started carries none, and none means zero. + * + * NOT `from.botId`. The assertion proves what this run is, and the Bot is whichever one the + * runtime is building right now: on a hop those agree, and taking the id from the signed + * value rather than from the build would let a stale assertion aim the next hop at the + * wrong Bot's grants. + */ + from: run, + // Read now rather than at boot, so a grant made a minute ago counts and one revoked a + // minute ago stops counting. + hasSomebodyToAsk: + ( + await pluginStore + .botsReachableFrom(botId) + .catch(() => [] as string[]) + ).length > 0, + maxDepth: config.handoff.maxDepth, + maxPerRun: config.handoff.maxPerRun, + }) + : null; + /* + * The way to stop and ask is offered whether or not there is a Bot to hand to. + * + * It is the cheaper of the two and the one a Bot should reach for first: asking the person who + * is already in the conversation spends nothing and cannot be aimed anywhere they cannot see. + * A deployment that offered only the expensive exit would push every unanswerable question + * sideways into another run. + */ + const asking = escalationTool({ + from: run, + route: askTheirOwnPerson, + auditStore: bootAuditStore, + }); + return passing ? [passing, asking] : [asking]; + }; + +/** + * One place a coworker is built for one person, for every surface that runs one. + * + * The named constants above exist because two callers had to build the SAME Bot. There are now + * four — a person's chat request, a routine's headless turn, a hop delivered to another Bot, and a + * Slack thread — and passing eleven collaborators to each of them in the right order is a drift + * waiting to happen: a surface that got one argument wrong would run, and quietly hold different + * tools or a different role from the Bot the person is talking to. So the collaborators are bound + * once here, and every surface asks this for a coworker instead. + */ +const actorAgentResolver = createActorAgentResolver({ + loadAgents: loadAgentsForActor, + model: tenantPackage.model, + resolveModelApiKey: resolveRuntimeModelApiKey, + stallGuard, + loadToolsForActor, + signRunForActor, + /* + * Only when a computer exists. The tools themselves are registered by the surface, so a Bot is + * offered them without this and the guidance is what tells it how they go together: snapshot + * before acting, and ask a person to take the wheel at a sign-in rather than reporting the task + * as impossible. Absent computer, absent guidance: a Bot is not told about hands it has not got. + */ + computerGuidance: config.computer ? COMPUTER_GUIDANCE : undefined, + loadVendors, + selectionForActor, + agentFetch, + handoffForActor, +}); + /** * Who a routine acts as, resolved the way {@link resolveRequestActor} resolves it. * @@ -636,24 +745,12 @@ const buildAgentFor = async ({ agentId: string; }) => { const actor = await actorFor(ownerUserId); - const agents = await resolveRuntimeAgents( - () => loadAgentsForActor(actor), - tenantPackage.model, - resolveRuntimeModelApiKey, - stallGuard, - loadToolsForActor(actor.id), - signRunForActor(actor.id), - config.computer ? COMPUTER_GUIDANCE : undefined, - loadVendors, - selectionForActor(actor.id), - agentFetch, - undefined, - // Only the Bot this routine names. Same reason as the hop delivery: the roster is still read in - // full so a Bot this owner cannot see is still absent, but the other Bots are neither built nor - // asked what they hold. - agentId, - ); - const agent = agents[agentId]; + // Only the Bot this routine names. Same reason as the hop delivery: the roster is still read in + // full so a Bot this owner cannot see is still absent, but the other Bots are neither built nor + // asked what they hold. + const agent = await actorAgentResolver + .resolveAgentForActor(actor, agentId) + .catch(() => null); if (!agent) { /* * Named, and raised rather than swallowed. The routine's Bot was deleted, or made private by @@ -717,93 +814,9 @@ const routineRunner = createRoutineRunner({ */ const copilotRuntime = mountCopilotRuntime( config, - tenantPackage.model, - loadAgentsForActor, - resolveRuntimeModelApiKey, + actorAgentResolver, identifyUser, identifyActor, - stallGuard, - loadToolsForActor, - signRunForActor, - undefined, - loadVendors, - selectionForActor, - agentFetch, - /* - * What a Bot may reach past itself for: another Bot, and a person. Made per run and per person. - * - * Per person because which Bots may be reached is decided against the roster that person can - * see: a Bot must never be able to address one they cannot, or this becomes a way around agent - * visibility. Per run because the caps need to know how deep the chain already is and where an - * answer belongs, and both of those are the deployment's own statement about the run rather than - * anything the model can edit. - */ - (actorId) => async (botId, input) => { - const from = readRunAssertion( - (input.forwardedProps as { openbotRun?: unknown } | undefined) - ?.openbotRun, - config.keyEncryptionKey, - ); - const run = { - botId, - actorId, - runId: input.runId, - threadId: input.threadId, - depth: from?.depth ?? 0, - }; - /* - * The caps are checked BEFORE the grants query, not inside the tool that would discard it. - * - * `handoffTool` short-circuits on all three of these, but only after being handed a - * `hasSomebodyToAsk` that costs a query. So a deployment which switched the capability off - * still paid one grants read per run of every Bot, for a tool it was never going to be offered, - * and a run already at the cap paid it again. - */ - const couldHandOn = - config.handoff.maxDepth > 0 && - config.handoff.maxPerRun > 0 && - run.depth < config.handoff.maxDepth; - - const passing = couldHandOn - ? handoffTool({ - desk: handoffDesk, - /* - * How deep this run already is comes from the assertion the deployment signed when it handed - * this work on. A run a person started carries none, and none means zero. - * - * NOT `from.botId`. The assertion proves what this run is, and the Bot is whichever one the - * runtime is building right now: on a hop those agree, and taking the id from the signed - * value rather than from the build would let a stale assertion aim the next hop at the - * wrong Bot's grants. - */ - from: run, - // Read now rather than at boot, so a grant made a minute ago counts and one revoked a - // minute ago stops counting. - hasSomebodyToAsk: - ( - await pluginStore - .botsReachableFrom(botId) - .catch(() => [] as string[]) - ).length > 0, - maxDepth: config.handoff.maxDepth, - maxPerRun: config.handoff.maxPerRun, - }) - : null; - /* - * The way to stop and ask is offered whether or not there is a Bot to hand to. - * - * It is the cheaper of the two and the one a Bot should reach for first: asking the person who - * is already in the conversation spends nothing and cannot be aimed anywhere they cannot see. - * A deployment that offered only the expensive exit would push every unanswerable question - * sideways into another run. - */ - const asking = escalationTool({ - from: run, - route: askTheirOwnPerson, - auditStore: bootAuditStore, - }); - return passing ? [passing, asking] : [asking]; - }, ); /** diff --git a/server/src/routing/routes.ts b/server/src/routing/routes.ts index 637713a3..c37b9754 100644 --- a/server/src/routing/routes.ts +++ b/server/src/routing/routes.ts @@ -1,96 +1,21 @@ import type { MiddlewareHandler } from "hono"; import { Hono } from "hono"; -import type { AgentProfileStore } from "../agents/profile-store"; -import type { AuditStore } from "../audit"; -import { recordAuditEvent } from "../audit"; import type { AppVariables } from "../auth/guards"; -import type { - IntentRouter, - RoutingCandidate, - RoutingUndecided, -} from "./classify"; - -const DEV_ACTOR_EMAIL = "dev@openbot.local"; +import type { HttpCoworkerRoutingService } from "./service"; /** - * Who to record the routing against, or nobody. + * Translate the shared coworker-routing result into the established HTTP contract. * - * The single-user development actor is not a real person and has no row to point at, so it is left - * off rather than written as a user id that resolves to nothing. - */ -function actorId( - actor: - | { - id?: string; - email?: string; - } - | null - | undefined, -): string | undefined { - return actor?.id && actor.email !== DEV_ACTOR_EMAIL ? actor.id : undefined; -} - -/** - * Decide which coworker a message is for, before a channel is pinned to one. - * - * The roster is read for the person asking, so this can only ever land on a coworker they are - * already allowed to reach. The decision is recorded like every other one in the product: a - * `channel.routed` row names where it went and why, and carries the candidate ids but never the - * message itself, which the audit payload redaction would drop anyway. - * - * A person who named a coworker with `@` has already decided, so nothing is inferred and no model - * is called. It is still recorded, with `viaMention` true and the person as the reason. Without - * that the trail answered "why did this go to Risk Analyst" for routed conversations and said - * nothing at all for chosen ones, which reads exactly like a row that failed to write. + * Choosing a coworker, applying visibility, invoking the intent model, and recording the canonical + * audit row are deliberately owned by CoworkerRoutingService. This layer only validates HTTP input + * and turns its outcome into status codes and JSON. */ export function createRoutingRoutes( - store: AgentProfileStore, - router: IntentRouter, + routing: HttpCoworkerRoutingService, requireUser: MiddlewareHandler<{ Variables: AppVariables }>, - auditStore?: AuditStore, - /** - * Which systems a coworker can reach, for the router to weigh alongside what it is for. - * - * Optional, and absent leaves routing exactly as it was: a deployment with no connectors has - * nothing to add here, and one that cannot answer the question should not have routing fail over - * it. Asked per request rather than held, because a grant added a minute ago has to count. - */ - reachableSystems?: (agentId: string) => Promise, ) { const routes = new Hono<{ Variables: AppVariables }>(); - /* - * The one place a `channel.routed` row is written, for both ways a message finds a coworker. - * - * Two call sites writing the same event is two payloads that drift, and a trail whose rows mean - * slightly different things depending on which branch produced them cannot be read at all. - */ - async function record( - actorUserId: string | undefined, - chosen: string, - reason: string, - fallback: boolean, - viaMention: boolean, - candidates: readonly string[], - /* - * Why the router did not decide, when it did not. - * - * On the row rather than only in the sentence, because this is the field a deployment counts. A - * router that has been unreachable for a week produced rows that read like ordinary - * "no confident match" ones, which is how #178 went unnoticed for as long as it did. - */ - undecided: RoutingUndecided | null, - ): Promise { - if (!auditStore) return; - await recordAuditEvent(auditStore, { - eventType: "channel.routed", - targetType: "agent", - targetId: chosen, - ...(actorUserId ? { actorUserId } : {}), - payload: { chosen, reason, fallback, viaMention, candidates, undecided }, - }); - } - routes.post("/", requireUser, async (context) => { const body = (await context.req.json().catch(() => null)) as { text?: unknown; @@ -98,104 +23,48 @@ export function createRoutingRoutes( } | null; const text = typeof body?.text === "string" ? body.text.trim() : ""; if (!text) return context.json({ error: "A message is required." }, 400); - const named = + const agentId = typeof body?.agentId === "string" && body.agentId.trim() ? body.agentId.trim() : null; - const actor = context.var.actor; - const roster = await store.list(actor, false); - // The same default the composer shows: the first public coworker, else the first at all. - const preferred = - roster.find((a) => a.visibility === "public") ?? roster[0]; - if (!preferred) { - return context.json({ error: "No coworker is available." }, 409); + const detail = await routing.routeDetailed({ + actor: context.var.actor, + text, + agentId, + }); + const { result } = detail; + if (result.kind === "none") { + return context.json( + { + error: agentId + ? "That coworker is not on your roster." + : "No coworker is available.", + }, + agentId ? 404 : 409, + ); } - - /* - * A named coworker is an instruction, not a question, so it is honoured as given. - * - * Checked against the same roster the router picks from, so `@` cannot reach further than - * routing can: a name that is not on it is refused rather than quietly turned into somebody - * else, because silently redirecting a message the person addressed by hand is the worst - * available answer. - */ - if (named) { - const chosen = roster.find((a) => a.id === named); - if (!chosen) { - return context.json( - { error: "That coworker is not on your roster." }, - 404, - ); - } - /* - * Third person, because the audit page is not read by the person who chose. - * - * This said "you chose them yourself", which is true in the conversation and false on an - * administrator's screen, where every row is somebody else's. The person is already on the - * row as `actorUserId`; the reason only has to say what kind of decision it was. - * - * @zopeVaibhav had this right in #134. - */ - const reason = "named by the person asking"; - // The person chose. Nothing was left to the router, so nothing about it was undecided. - await record( - actorId(actor), - chosen.id, - reason, - false, - true, - [chosen.id], - null, + if (result.kind === "ambiguous") { + return context.json( + { + error: "More than one coworker matches that name.", + names: result.names, + }, + 409, ); - return context.json({ - agentId: chosen.id, - name: chosen.name, - reason, - fallback: false, - viaMention: true, - }); } - const candidates: RoutingCandidate[] = await Promise.all( - roster.map(async (a) => ({ - id: a.id, - name: a.name, - roleDescription: a.roleDescription, - /* - * Never allowed to break routing. A connector store that is slow or unhappy must not turn - * "who is this for" into an error, so a failure here is the same as holding nothing: the - * router falls back to matching on purpose alone, which is what it did before. - */ - ...(reachableSystems - ? { - reaches: await reachableSystems(a.id).catch( - () => [] as readonly string[], - ), - } - : {}), - })), - ); - - const decision = await router.route(text, candidates, preferred.id); - - await record( - actorId(actor), - decision.agentId, - decision.reason, - decision.fallback, - false, - candidates.map((c) => c.id), - decision.undecided, + const response = { + agentId: result.agentId, + name: result.name, + reason: result.reason, + fallback: result.fallback, + viaMention: result.viaMention, + }; + // The composer chose this coworker directly, and the legacy response did not expose a model + // fallback cause for that path. Keep the model-routed response shape unchanged below. + return context.json( + agentId ? response : { ...response, undecided: detail.undecided }, ); - - return context.json({ - agentId: decision.agentId, - name: decision.name, - reason: decision.reason, - fallback: decision.fallback, - undecided: decision.undecided, - viaMention: false, - }); }); return routes; diff --git a/server/src/routing/service.ts b/server/src/routing/service.ts new file mode 100644 index 00000000..c9dadf78 --- /dev/null +++ b/server/src/routing/service.ts @@ -0,0 +1,382 @@ +import { canAccessAgent } from "../agents/profile-policy"; +import type { AgentProfileStore } from "../agents/profile-store"; +import type { AgentActor, AgentProfile } from "../agents/profile-types"; +import type { AuditStore } from "../audit"; +import { recordAuditEvent } from "../audit"; +import type { + IntentRouter, + RoutingCandidate, + RoutingUndecided, +} from "./classify"; + +const DEV_ACTOR_EMAIL = "dev@openbot.local"; +const WORD_CHARACTER = /[\p{L}\p{N}\p{M}_]/u; + +export type CoworkerRouteResult = + | { + kind: "selected"; + agentId: string; + name: string; + reason: string; + fallback: boolean; + viaMention: boolean; + } + | { kind: "ambiguous"; names: string[] } + | { kind: "none" }; + +type RoutingActor = AgentActor & { email?: string }; + +export type CoworkerRoutingInput = { + actor: RoutingActor; + text: string; + /** An explicit picker selection from a surface such as the web composer. */ + agentId?: string | null; +}; + +export type CoworkerRouteDetail = { + result: CoworkerRouteResult; + /** Kept for surfaces that have historically returned the model's fallback cause. */ + undecided: RoutingUndecided | null; +}; + +export type CoworkerRoutingService = { + route(input: CoworkerRoutingInput): Promise; +}; + +export type HttpCoworkerRoutingService = CoworkerRoutingService & { + routeDetailed(input: CoworkerRoutingInput): Promise; +}; + +export type CreateCoworkerRoutingServiceOptions = { + store: AgentProfileStore; + router: IntentRouter; + auditStore?: AuditStore; + reachableSystems?: (agentId: string) => Promise; +}; + +/** A safe categorical failure for a roster whose connector reachability could not be checked. */ +export class CoworkerReachabilityUnavailableError extends Error { + readonly code = "coworker_reachability_unavailable"; + + constructor() { + super("Coworker reachability is temporarily unavailable"); + this.name = "CoworkerReachabilityUnavailableError"; + } +} + +/** Normalize people-facing names before matching, without making matching fuzzy. */ +export function normalizeCoworkerName(value: string): string { + return value.normalize("NFKC").toLowerCase().trim().replace(/\s+/gu, " "); +} + +function hasTokenBoundaries(text: string, start: number, end: number): boolean { + const before = [...text.slice(0, start)].at(-1); + const after = [...text.slice(end)][0]; + return !before || !WORD_CHARACTER.test(before) + ? !after || !WORD_CHARACTER.test(after) + : false; +} + +type AliasOccurrence = { + start: number; + end: number; + profiles: ReadonlyMap; +}; + +function occurrencesOf( + text: string, + alias: string, + profiles: ReadonlyMap, +): AliasOccurrence[] { + const occurrences: AliasOccurrence[] = []; + let start = text.indexOf(alias); + while (start >= 0) { + const end = start + alias.length; + if (hasTokenBoundaries(text, start, end)) { + occurrences.push({ start, end, profiles }); + } + start = text.indexOf(alias, start + alias.length); + } + return occurrences; +} + +function actorId(actor: RoutingActor): string | undefined { + return actor.id && actor.email !== DEV_ACTOR_EMAIL ? actor.id : undefined; +} + +function suffixes(name: string): string[] { + const tokens = name.split(" "); + return tokens.slice(1).map((_, index) => tokens.slice(index + 1).join(" ")); +} + +function displayName(name: string): string { + return name.normalize("NFKC").trim().replace(/\s+/gu, " "); +} + +function utf8Hex(value: string): string { + return [...new TextEncoder().encode(value)] + .map((byte) => byte.toString(16).padStart(2, "0")) + .join(""); +} + +function codePointCompare(left: string, right: string): number { + const leftPoints = [...left]; + const rightPoints = [...right]; + for (let index = 0; index < leftPoints.length; index += 1) { + const leftPoint = leftPoints[index]?.codePointAt(0); + const rightPoint = rightPoints[index]?.codePointAt(0); + if (leftPoint === undefined) return -1; + if (rightPoint === undefined) return 1; + if (leftPoint !== rightPoint) return leftPoint - rightPoint; + } + return leftPoints.length - rightPoints.length; +} + +type AliasIndex = { + aliases: Map>; + labels: Map; +}; + +function addAlias( + aliases: AliasIndex["aliases"], + alias: string, + profile: AgentProfile, +): void { + if (!alias) return; + const profiles = aliases.get(alias) ?? new Map(); + profiles.set(profile.id, profile); + aliases.set(alias, profiles); +} + +function buildAliasIndex(roster: readonly AgentProfile[]): AliasIndex { + const byNormalizedName = new Map(); + for (const profile of roster) { + const normalized = normalizeCoworkerName(profile.name); + const profiles = byNormalizedName.get(normalized); + if (profiles) profiles.push(profile); + else byNormalizedName.set(normalized, [profile]); + } + + const aliases = new Map>(); + const labels = new Map(); + for (const profile of roster) { + const normalized = normalizeCoworkerName(profile.name); + const duplicates = byNormalizedName.get(normalized) ?? []; + const label = + duplicates.length > 1 + ? `${displayName(profile.name)} (id ${utf8Hex(profile.id)})` + : profile.name; + labels.set(profile.id, label); + addAlias(aliases, normalized, profile); + for (const suffix of suffixes(normalized)) + addAlias(aliases, suffix, profile); + if (duplicates.length > 1) { + addAlias(aliases, normalizeCoworkerName(label), profile); + } + } + return { aliases, labels }; +} + +type ExplicitOccurrence = { + start: number; + end: number; + profiles: Map; +}; + +/** + * Discard only aliases that a strictly longer explicit occurrence fully contains. + * + * Intervals are ordered by start, then widest first. A running maximum end therefore proves that a + * prior interval starts no later and reaches at least as far as the current one, which is exactly + * containment. Partial overlaps extend the maximum only for later contained intervals; they never + * suppress each other. + */ +function withoutContainedOccurrences( + occurrences: readonly AliasOccurrence[], +): ExplicitOccurrence[] { + const bySpan = new Map(); + for (const occurrence of occurrences) { + const key = `${occurrence.start}:${occurrence.end}`; + const merged = + bySpan.get(key) ?? + ({ + start: occurrence.start, + end: occurrence.end, + profiles: new Map(), + } satisfies ExplicitOccurrence); + for (const profile of occurrence.profiles.values()) { + merged.profiles.set(profile.id, profile); + } + bySpan.set(key, merged); + } + + const sorted = [...bySpan.values()].sort( + (left, right) => left.start - right.start || right.end - left.end, + ); + let maximumEnd = -1; + return sorted.filter((occurrence) => { + const contained = maximumEnd >= occurrence.end; + maximumEnd = Math.max(maximumEnd, occurrence.end); + return !contained; + }); +} + +function labelsFor( + profiles: Iterable, + labels: ReadonlyMap, +): string[] { + return [...profiles] + .map((profile) => labels.get(profile.id) ?? profile.name) + .sort(codePointCompare); +} + +function explicitNameRoute( + text: string, + roster: readonly AgentProfile[], +): CoworkerRouteResult | null { + const normalizedText = normalizeCoworkerName(text); + const { aliases, labels } = buildAliasIndex(roster); + const occurrences = [...aliases.entries()].flatMap(([alias, profiles]) => + occurrencesOf(normalizedText, alias, profiles), + ); + const explicitOccurrences = withoutContainedOccurrences(occurrences); + const profiles = new Map(); + for (const occurrence of explicitOccurrences) { + for (const profile of occurrence.profiles.values()) { + profiles.set(profile.id, profile); + } + } + if (profiles.size === 1) { + const chosen = profiles.values().next().value as AgentProfile; + return { + kind: "selected", + agentId: chosen.id, + name: chosen.name, + reason: "named by the person asking", + fallback: false, + viaMention: true, + }; + } + if (profiles.size > 1) { + return { kind: "ambiguous", names: labelsFor(profiles.values(), labels) }; + } + return null; +} + +function auditReason( + selected: Extract, + undecided: RoutingUndecided | null, +): string { + if (selected.viaMention) return "named by the person asking"; + if (selected.fallback) + return undecided ? `fallback: ${undecided}` : "fallback"; + return "intent match"; +} + +export function createCoworkerRoutingService( + options: CreateCoworkerRoutingServiceOptions, +): HttpCoworkerRoutingService { + async function record( + actor: RoutingActor, + selected: Extract, + candidates: readonly string[], + undecided: RoutingUndecided | null, + ): Promise { + if (!options.auditStore) return; + await recordAuditEvent(options.auditStore, { + eventType: "channel.routed", + targetType: "agent", + targetId: selected.agentId, + ...(actorId(actor) ? { actorUserId: actorId(actor) } : {}), + payload: { + chosen: selected.agentId, + reason: auditReason(selected, undecided), + fallback: selected.fallback, + viaMention: selected.viaMention, + candidates, + undecided, + }, + }); + } + + async function routeDetailed( + input: CoworkerRoutingInput, + ): Promise { + // The store applies this same policy in SQL; keep this canonical policy check at the service + // boundary so a broader store implementation cannot leak a coworker into routing. + const roster = (await options.store.list(input.actor, false)).filter( + (profile) => canAccessAgent(input.actor, profile), + ); + const namedId = input.agentId?.trim() || null; + if (namedId) { + const chosen = roster.find(({ id }) => id === namedId); + if (!chosen) return { result: { kind: "none" }, undecided: null }; + const result: Extract = { + kind: "selected", + agentId: chosen.id, + name: chosen.name, + reason: "named by the person asking", + fallback: false, + viaMention: true, + }; + await record(input.actor, result, [chosen.id], null); + return { result, undecided: null }; + } + + if (roster.length === 0) + return { result: { kind: "none" }, undecided: null }; + + const explicit = explicitNameRoute(input.text, roster); + if (explicit) { + if (explicit.kind === "selected") { + await record(input.actor, explicit, [explicit.agentId], null); + } + return { result: explicit, undecided: null }; + } + + const preferred = + roster.find(({ visibility }) => visibility === "public") ?? roster[0]; + if (!preferred) return { result: { kind: "none" }, undecided: null }; + const candidates: RoutingCandidate[] = await Promise.all( + roster.map(async (profile) => ({ + id: profile.id, + name: profile.name, + roleDescription: profile.roleDescription, + ...(options.reachableSystems + ? { + reaches: await options.reachableSystems(profile.id).catch(() => { + throw new CoworkerReachabilityUnavailableError(); + }), + } + : {}), + })), + ); + const decision = await options.router.route( + input.text, + candidates, + preferred.id, + ); + const result: Extract = { + kind: "selected", + agentId: decision.agentId, + name: decision.name, + reason: decision.reason, + fallback: decision.fallback, + viaMention: false, + }; + await record( + input.actor, + result, + candidates.map(({ id }) => id), + decision.undecided, + ); + return { result, undecided: decision.undecided }; + } + + return { + async route(input) { + return (await routeDetailed(input)).result; + }, + routeDetailed, + }; +} diff --git a/server/tests/agent-resolver.test.ts b/server/tests/agent-resolver.test.ts new file mode 100644 index 00000000..53bfd746 --- /dev/null +++ b/server/tests/agent-resolver.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, test } from "bun:test"; +import { BuiltInAgent } from "@copilotkit/runtime/v2"; +import { createActorAgentResolver } from "../src/agents/agent-resolver"; + +describe("actor-scoped agent resolver", () => { + test("uses the same actor for web maps and individual agent resolution", async () => { + const seenActorIds: string[] = []; + const resolver = createActorAgentResolver({ + loadAgents: async (actor) => { + seenActorIds.push(actor.id); + return [ + { + id: "risk", + name: "Risk Analyst", + type: "built_in" as const, + systemPrompt: "Assess operational risk.", + }, + ]; + }, + model: { provider: "openai", defaultModel: "gpt-5.6-terra" }, + resolveModelApiKey: async () => "openai-secret", + }); + const actor = { id: "u1", role: "user" as const }; + + const visibleAgents = await resolver.resolveAgentsForActor(actor); + const risk = await resolver.resolveAgentForActor(actor, "risk"); + + expect(seenActorIds).toEqual(["u1", "u1"]); + expect(visibleAgents.risk).toBeInstanceOf(BuiltInAgent); + expect(risk).toBeInstanceOf(BuiltInAgent); + }); + + test("rejects an agent absent from the actor's visible map", async () => { + const resolver = createActorAgentResolver({ + loadAgents: async () => [ + { + id: "risk", + name: "Risk Analyst", + type: "built_in" as const, + systemPrompt: "Assess operational risk.", + }, + ], + model: { provider: "openai", defaultModel: "gpt-5.6-terra" }, + resolveModelApiKey: async () => "openai-secret", + }); + + let rejection: unknown; + try { + await resolver.resolveAgentForActor( + { id: "u1", role: "user" }, + "private-risk", + ); + } catch (error) { + rejection = error; + } + + expect(rejection).toBeInstanceOf(Error); + expect((rejection as Error).message).toBe( + "Coworker private-risk is unavailable to this user.", + ); + }); + + test("rejects an agent when the actor has no visible coworkers", async () => { + const resolver = createActorAgentResolver({ + loadAgents: async () => [], + model: { provider: "openai", defaultModel: "gpt-5.6-terra" }, + resolveModelApiKey: async () => "openai-secret", + }); + + expect( + await rejectionMessage(() => + resolver.resolveAgentForActor( + { id: "u1", role: "user" }, + "private-risk", + ), + ), + ).toBe("Coworker private-risk is unavailable to this user."); + }); + + test("rejects inherited object keys as unavailable coworkers", async () => { + const resolver = createActorAgentResolver({ + loadAgents: async () => [ + { + id: "risk", + name: "Risk Analyst", + type: "built_in" as const, + systemPrompt: "Assess operational risk.", + }, + ], + model: { provider: "openai", defaultModel: "gpt-5.6-terra" }, + resolveModelApiKey: async () => "openai-secret", + }); + + for (const agentId of ["constructor", "toString", "__proto__"]) { + expect( + await rejectionMessage(() => + resolver.resolveAgentForActor({ id: "u1", role: "user" }, agentId), + ), + ).toBe(`Coworker ${agentId} is unavailable to this user.`); + } + }); +}); + +async function rejectionMessage(run: () => Promise) { + try { + await run(); + } catch (error) { + if (error instanceof Error) return error.message; + throw error; + } + throw new Error("Expected the run to reject."); +} diff --git a/server/tests/copilot.test.ts b/server/tests/copilot.test.ts index 6d439464..e362c4a8 100644 --- a/server/tests/copilot.test.ts +++ b/server/tests/copilot.test.ts @@ -2,6 +2,7 @@ import { describe, expect, spyOn, test } from "bun:test"; import { HttpAgent } from "@ag-ui/client"; import { BuiltInAgent } from "@copilotkit/runtime/v2"; import { PROVENANCE_GUIDANCE } from "../../shared/bot-prompt"; +import { createActorAgentResolver } from "../src/agents/agent-resolver"; import { buildAgents, builtInAgentConfiguration, @@ -518,12 +519,14 @@ describe("standing agent roles", () => { seen.request = request; return { id: "user-7", role: "user" as const }; }, - async (actor) => { - seen.actors.push(actor); - return [remoteAgent("http://coworker.internal/ag-ui")]; - }, - { provider: "openai", defaultModel: "gpt-5.6-terra" }, - async () => null, + createActorAgentResolver({ + loadAgents: async (actor) => { + seen.actors.push(actor); + return [remoteAgent("http://coworker.internal/ag-ui")]; + }, + model: { provider: "openai", defaultModel: "gpt-5.6-terra" }, + resolveModelApiKey: async () => null, + }), ); const request = new Request("http://openbot.test/api/copilotkit"); @@ -538,11 +541,13 @@ describe("standing agent roles", () => { let roleDescription = "Review receipts."; const factory = createRequestAgents( async () => ({ id: "user-7", role: "user" as const }), - async () => [ - remoteAgent("http://coworker.internal/ag-ui", { roleDescription }), - ], - { provider: "openai", defaultModel: "gpt-5.6-terra" }, - async () => null, + createActorAgentResolver({ + loadAgents: async () => [ + remoteAgent("http://coworker.internal/ag-ui", { roleDescription }), + ], + model: { provider: "openai", defaultModel: "gpt-5.6-terra" }, + resolveModelApiKey: async () => null, + }), ); const request = new Request("http://openbot.test/api/copilotkit"); diff --git a/server/tests/routing-routes.test.ts b/server/tests/routing-routes.test.ts index b974d136..5ac6df66 100644 --- a/server/tests/routing-routes.test.ts +++ b/server/tests/routing-routes.test.ts @@ -6,6 +6,7 @@ import type { AuditStore } from "../src/audit"; import type { AppVariables } from "../src/auth/guards"; import type { IntentRouter, RoutingUndecided } from "../src/routing/classify"; import { createRoutingRoutes } from "../src/routing/routes"; +import { createCoworkerRoutingService } from "../src/routing/service"; /** * Why a conversation went where it went, for every conversation. @@ -32,12 +33,16 @@ const ROSTER = [ name: "Risk Analyst", roleDescription: "regulatory and compliance questions", visibility: "public", + ownerUserId: null, + deletedAt: null, }, { id: "knowledge", name: "Knowledge", roleDescription: "company knowledge", visibility: "public", + ownerUserId: null, + deletedAt: null, }, ]; @@ -87,7 +92,10 @@ function app(options: { routed?: string; undecided?: RoutingUndecided } = {}) { const server = new Hono<{ Variables: AppVariables }>(); server.route( "/api/route", - createRoutingRoutes(store, router, asActor, auditStore), + createRoutingRoutes( + createCoworkerRoutingService({ store, router, auditStore }), + asActor, + ), ); return { server, written, asked }; } @@ -113,9 +121,10 @@ describe("recording which coworker a message went to", () => { }); expect(response.status).toBe(200); - expect(await response.json()).toMatchObject({ + expect(await response.json()).toEqual({ agentId: "risk-analyst", name: "Risk Analyst", + reason: "named by the person asking", viaMention: true, fallback: false, }); diff --git a/server/tests/routing-service.test.ts b/server/tests/routing-service.test.ts new file mode 100644 index 00000000..4b039b50 --- /dev/null +++ b/server/tests/routing-service.test.ts @@ -0,0 +1,576 @@ +import { describe, expect, test } from "bun:test"; +import type { AgentProfileStore } from "../src/agents/profile-store"; +import type { AgentProfile } from "../src/agents/profile-types"; +import type { AuditStore } from "../src/audit"; +import type { + IntentRouter, + RoutingCandidate, + RoutingUndecided, +} from "../src/routing/classify"; +import { + createCoworkerRoutingService, + normalizeCoworkerName, +} from "../src/routing/service"; + +const ACTOR = { id: "u1", role: "user" } as const; + +function profile( + id: string, + name: string, + visibility: "public" | "private" = "public", + ownerUserId: string | null = null, +): AgentProfile { + return { + id, + name, + title: name, + roleDescription: `${name} work`, + avatarSeed: id, + visibility, + ownerUserId, + systemOwned: false, + hidden: false, + deletedAt: null, + endpoint: null, + hasAuth: false, + hasCallbackToken: false, + }; +} + +function makeService( + options: { + roster?: AgentProfile[]; + decision?: { + agentId: string; + reason: string; + fallback: boolean; + undecided: RoutingUndecided | null; + }; + reachableSystems?: (agentId: string) => Promise; + } = {}, +) { + const roster = options.roster ?? [ + profile("risk", "Risk Analyst"), + profile("knowledge", "Knowledge"), + ]; + const modelCalls: Array<{ + text: string; + candidates: readonly RoutingCandidate[]; + defaultId: string; + }> = []; + const audits: Array<{ + payload: Record; + targetId: string | null; + }> = []; + const store = { list: async () => roster } as unknown as AgentProfileStore; + const router = { + route: async ( + text: string, + candidates: readonly RoutingCandidate[], + defaultId: string, + ) => { + modelCalls.push({ text, candidates, defaultId }); + const selected = options.decision ?? { + agentId: "knowledge", + reason: "matches what it is for", + fallback: false, + undecided: null, + }; + const candidate = candidates.find(({ id }) => id === selected.agentId); + return { ...selected, name: candidate?.name ?? selected.agentId }; + }, + } as unknown as IntentRouter; + const auditStore = { + insert: async (event: { + payload: Record; + targetId: string | null; + }) => { + audits.push(event); + }, + } as unknown as AuditStore; + + return { + service: createCoworkerRoutingService({ + store, + router, + auditStore, + reachableSystems: options.reachableSystems, + }), + modelCalls, + audits, + }; +} + +describe("CoworkerRoutingService", () => { + test("routes a unique explicit coworker name without invoking the model", async () => { + const { service, modelCalls } = makeService(); + + const result = await service.route({ + actor: ACTOR, + text: "ask risk analyst to review this", + }); + + expect(result).toMatchObject({ + kind: "selected", + agentId: "risk", + viaMention: true, + }); + expect(modelCalls).toEqual([]); + }); + + test("normalizes explicit names with NFKC, case, and whitespace", async () => { + const { service, modelCalls } = makeService({ + roster: [profile("risk", "Ｒｉｓｋ Analyst")], + }); + + expect( + await service.route({ actor: ACTOR, text: "Ask risk\tanalyst please" }), + ).toMatchObject({ + kind: "selected", + agentId: "risk", + viaMention: true, + }); + expect(modelCalls).toEqual([]); + }); + + test("does not match a coworker name inside a larger word", async () => { + const { service, modelCalls } = makeService({ + roster: [profile("risk", "Risk")], + }); + + await service.route({ actor: ACTOR, text: "de-risking the portfolio" }); + + expect(modelCalls).toHaveLength(1); + }); + + test("uses Unicode token boundaries instead of ASCII word boundaries", async () => { + const { service, modelCalls } = makeService({ + roster: [profile("risk", "Risk")], + }); + + await service.route({ actor: ACTOR, text: "Risk\u{10400} review" }); + + expect(modelCalls).toHaveLength(1); + }); + + test("returns visible choices for an ambiguous explicit name", async () => { + const { service, modelCalls } = makeService({ + roster: [ + profile("risk", "Risk Analyst"), + profile("data", "Data Analyst"), + ], + }); + + expect( + await service.route({ actor: ACTOR, text: "ask analyst to review this" }), + ).toEqual({ + kind: "ambiguous", + names: ["Data Analyst", "Risk Analyst"], + }); + expect(modelCalls).toEqual([]); + }); + + test("treats a nested full name as ambiguous when its alias belongs to another coworker", async () => { + const { service, modelCalls } = makeService({ + roster: [profile("analyst", "Analyst"), profile("risk", "Risk Analyst")], + }); + + expect( + await service.route({ actor: ACTOR, text: "ask analyst to review this" }), + ).toEqual({ + kind: "ambiguous", + names: ["Analyst", "Risk Analyst"], + }); + expect(modelCalls).toEqual([]); + }); + + test("prefers a unique longer explicit alias over a shared suffix", async () => { + const { service, modelCalls } = makeService({ + roster: [profile("analyst", "Analyst"), profile("risk", "Risk Analyst")], + }); + + expect( + await service.route({ + actor: ACTOR, + text: "ask risk analyst to review this", + }), + ).toMatchObject({ + kind: "selected", + agentId: "risk", + viaMention: true, + }); + expect(modelCalls).toEqual([]); + }); + + test("returns choices when two independent explicit names appear in long-to-short order", async () => { + const { service, modelCalls } = makeService(); + + expect( + await service.route({ + actor: ACTOR, + text: "ask Risk Analyst and Knowledge to review this", + }), + ).toEqual({ + kind: "ambiguous", + names: ["Knowledge", "Risk Analyst"], + }); + expect(modelCalls).toEqual([]); + }); + + test("returns choices when two independent explicit names appear in short-to-long order", async () => { + const { service, modelCalls } = makeService(); + + expect( + await service.route({ + actor: ACTOR, + text: "ask Knowledge and Risk Analyst to review this", + }), + ).toEqual({ + kind: "ambiguous", + names: ["Knowledge", "Risk Analyst"], + }); + expect(modelCalls).toEqual([]); + }); + + test("selects a profile when all explicit mentions refer to that same profile", async () => { + const { service, modelCalls } = makeService(); + + expect( + await service.route({ + actor: ACTOR, + text: "ask Risk Analyst and Risk Analyst to review this", + }), + ).toMatchObject({ kind: "selected", agentId: "risk", viaMention: true }); + expect(modelCalls).toEqual([]); + }); + + test("keeps partially overlapping full names as independent explicit choices", async () => { + const { service, modelCalls } = makeService({ + roster: [profile("ann", "Ann Marie"), profile("curie", "Marie Curie")], + }); + + expect( + await service.route({ actor: ACTOR, text: "ask Ann Marie Curie" }), + ).toEqual({ + kind: "ambiguous", + names: ["Ann Marie", "Marie Curie"], + }); + expect(modelCalls).toEqual([]); + }); + + test("suppresses contained prefix aliases but retains a partially overlapping suffix name", async () => { + const { service, modelCalls } = makeService({ + roster: [ + profile("ann", "Ann"), + profile("ann-marie", "Ann Marie"), + profile("curie", "Marie Curie"), + ], + }); + + expect( + await service.route({ actor: ACTOR, text: "ask Ann Marie Curie" }), + ).toEqual({ + kind: "ambiguous", + names: ["Ann Marie", "Marie Curie"], + }); + expect(modelCalls).toEqual([]); + }); + + test("handles many repeated explicit mentions without changing their selection", async () => { + const { service, modelCalls } = makeService(); + const text = Array.from({ length: 1_500 }, () => "Risk Analyst").join( + " and ", + ); + + expect(await service.route({ actor: ACTOR, text })).toMatchObject({ + kind: "selected", + agentId: "risk", + viaMention: true, + }); + expect(modelCalls).toEqual([]); + }); + + test("labels duplicate normalized names distinctly and resolves a chosen label", async () => { + const { service, modelCalls } = makeService({ + roster: [ + profile("risk-id", "Risk Analyst"), + profile("risk-copy", "Ｒｉｓｋ Analyst"), + ], + }); + + expect( + await service.route({ + actor: ACTOR, + text: "ask risk analyst to review this", + }), + ).toEqual({ + kind: "ambiguous", + names: [ + "Risk Analyst (id 7269736b2d636f7079)", + "Risk Analyst (id 7269736b2d6964)", + ], + }); + expect( + await service.route({ + actor: ACTOR, + text: "ask Risk Analyst (id 7269736b2d636f7079) to review this", + }), + ).toMatchObject({ + kind: "selected", + agentId: "risk-copy", + viaMention: true, + }); + expect(modelCalls).toEqual([]); + }); + + test("uses stable id labels when duplicate ids differ only by case", async () => { + const roster = [ + profile("risk", "Risk Analyst"), + profile("Risk", "Risk Analyst"), + ]; + const { service, modelCalls } = makeService({ roster }); + + const result = await service.route({ + actor: ACTOR, + text: "ask risk analyst to review this", + }); + expect(result).toEqual({ + kind: "ambiguous", + names: ["Risk Analyst (id 5269736b)", "Risk Analyst (id 7269736b)"], + }); + expect(new Set(result.names.map(normalizeCoworkerName)).size).toBe( + result.names.length, + ); + expect( + await service.route({ + actor: ACTOR, + text: "ask Risk Analyst (id 5269736b) to review this", + }), + ).toMatchObject({ kind: "selected", agentId: "Risk" }); + expect(modelCalls).toEqual([]); + }); + + test("uses NFKC-distinct id labels with an order-independent mapping", async () => { + const optionA = profile("A", "Risk Analyst"); + const optionFullWidthA = profile("Ａ", "Risk Analyst"); + const forward = makeService({ roster: [optionFullWidthA, optionA] }); + const reverse = makeService({ roster: [optionA, optionFullWidthA] }); + + const forwardResult = await forward.service.route({ + actor: ACTOR, + text: "ask risk analyst to review this", + }); + const reverseResult = await reverse.service.route({ + actor: ACTOR, + text: "ask risk analyst to review this", + }); + + expect(forwardResult).toEqual({ + kind: "ambiguous", + names: ["Risk Analyst (id 41)", "Risk Analyst (id efbca1)"], + }); + expect(reverseResult).toEqual(forwardResult); + expect( + await forward.service.route({ + actor: ACTOR, + text: "ask Risk Analyst (id 41) to review this", + }), + ).toMatchObject({ kind: "selected", agentId: "A" }); + expect( + await forward.service.route({ + actor: ACTOR, + text: "ask Risk Analyst (id efbca1) to review this", + }), + ).toMatchObject({ kind: "selected", agentId: "Ａ" }); + }); + + test("keeps a duplicate label bound to its id when new duplicates are added or reordered", async () => { + const idA = profile("a", "Risk Analyst"); + const idB = profile("b", "Risk Analyst"); + const addedEarlier = profile("A", "Risk Analyst"); + const original = makeService({ roster: [idB, idA] }); + const expanded = makeService({ roster: [idB, addedEarlier, idA] }); + const stableLabel = "Risk Analyst (id 62)"; + + expect( + await original.service.route({ + actor: ACTOR, + text: "ask risk analyst to review this", + }), + ).toEqual({ + kind: "ambiguous", + names: ["Risk Analyst (id 61)", stableLabel], + }); + expect( + await expanded.service.route({ + actor: ACTOR, + text: "ask risk analyst to review this", + }), + ).toEqual({ + kind: "ambiguous", + names: ["Risk Analyst (id 41)", "Risk Analyst (id 61)", stableLabel], + }); + expect( + await expanded.service.route({ + actor: ACTOR, + text: `ask ${stableLabel}`, + }), + ).toMatchObject({ kind: "selected", agentId: "b" }); + }); + + test("uses normalization-safe labels for base64url case collisions", async () => { + const first = profile("\u0800", "Risk Analyst"); + const second = profile("\u081A", "Risk Analyst"); + const { service } = makeService({ roster: [second, first] }); + + const result = await service.route({ + actor: ACTOR, + text: "ask risk analyst to review this", + }); + + expect(result).toEqual({ + kind: "ambiguous", + names: ["Risk Analyst (id e0a080)", "Risk Analyst (id e0a09a)"], + }); + expect(new Set(result.names.map(normalizeCoworkerName)).size).toBe( + result.names.length, + ); + expect( + await service.route({ + actor: ACTOR, + text: "ask Risk Analyst (id e0a080) to review this", + }), + ).toMatchObject({ kind: "selected", agentId: "\u0800" }); + expect( + await service.route({ + actor: ACTOR, + text: "ask Risk Analyst (id e0a09a) to review this", + }), + ).toMatchObject({ kind: "selected", agentId: "\u081A" }); + }); + + test("returns none for an absent or empty visible roster", async () => { + const { service } = makeService({ roster: [] }); + + expect(await service.route({ actor: ACTOR, text: "anything" })).toEqual({ + kind: "none", + }); + }); + + test("returns none when an explicit composer id is inaccessible", async () => { + const { service, modelCalls } = makeService({ + roster: [ + profile("risk", "Risk Analyst"), + profile("private", "Private Analyst", "private", "u2"), + ], + }); + + expect( + await service.route({ + actor: ACTOR, + text: "anything", + agentId: "private", + }), + ).toEqual({ kind: "none" }); + expect(modelCalls).toEqual([]); + }); + + test("falls back to intent routing when no explicit name appears", async () => { + const { service, modelCalls } = makeService(); + + expect( + await service.route({ actor: ACTOR, text: "what is our PTO policy" }), + ).toMatchObject({ + kind: "selected", + agentId: "knowledge", + viaMention: false, + }); + expect(modelCalls).toHaveLength(1); + }); + + test("fails safely when coworker reachability cannot be loaded", async () => { + const { service, modelCalls } = makeService({ + reachableSystems: async () => { + throw new Error("private reachability detail"); + }, + }); + + await expect( + service.route({ actor: ACTOR, text: "what is in Drive?" }), + ).rejects.toThrow("Coworker reachability is temporarily unavailable"); + expect(modelCalls).toEqual([]); + }); + + test("passes only the actor-visible roster to intent routing", async () => { + const visible = profile("mine", "My Private", "private", ACTOR.id); + const inaccessible = profile("other", "Other Private", "private", "u2"); + const deleted = { + ...profile("deleted", "Deleted", "public"), + deletedAt: new Date(), + }; + const { service, modelCalls } = makeService({ + roster: [profile("public", "Public"), visible, inaccessible, deleted], + }); + + await service.route({ actor: ACTOR, text: "anything" }); + + expect(modelCalls[0]?.candidates.map(({ id }) => id)).toEqual([ + "public", + "mine", + ]); + }); + + test("writes selected audit fields exactly once without message text", async () => { + const { service, audits } = makeService(); + + await service.route({ actor: ACTOR, text: "private payroll details" }); + + expect(audits).toHaveLength(1); + expect(audits[0]).toMatchObject({ targetId: "knowledge" }); + expect(audits[0]?.payload).toEqual({ + chosen: "knowledge", + reason: "intent match", + fallback: false, + viaMention: false, + candidates: ["risk", "knowledge"], + undecided: null, + }); + expect(JSON.stringify(audits[0])).not.toContain("private payroll details"); + }); + + test("preserves a model fallback audit cause", async () => { + const { service, audits } = makeService({ + decision: { + agentId: "knowledge", + reason: "sent to your default while the router was unreachable", + fallback: true, + undecided: "unreachable", + }, + }); + + await service.route({ actor: ACTOR, text: "anything" }); + + expect(audits[0]?.payload).toMatchObject({ + fallback: true, + undecided: "unreachable", + }); + }); + + test("does not persist a model reason that echoes the message", async () => { + const text = "private payroll details for Sam"; + const { service, audits } = makeService({ + decision: { + agentId: "knowledge", + reason: text, + fallback: false, + undecided: null, + }, + }); + + const result = await service.route({ actor: ACTOR, text }); + + expect(result).toMatchObject({ kind: "selected", reason: text }); + expect(audits[0]?.payload.reason).toBe("intent match"); + expect(JSON.stringify(audits[0])).not.toContain(text); + }); +});