diff --git a/CHANGELOG.md b/CHANGELOG.md index 296cb10ed..f856dbb9d 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 coworker created as Built in can hand work on + +On a deployment with a managed Bot, choosing **Built in** used to store the coworker at that Bot's +endpoint as `remote_ag_ui`. It could answer and be asked, but it could not hand work on, because the +handoff tool is minted only inside this deployment's own run loop. An endpoint-less create now stays +`built_in` and runs on its role description even when a managed Bot exists. On startup, coworkers +already stored at the configured managed endpoint are repaired the same way; rows with their own +authentication are left alone. + ### Bots work as coworkers A Bot can now carry on without anyone watching it. It runs standing **Responsibilities** fed by diff --git a/server/src/agents/profile-store.ts b/server/src/agents/profile-store.ts index 426bb67aa..6804d2acd 100644 --- a/server/src/agents/profile-store.ts +++ b/server/src/agents/profile-store.ts @@ -364,6 +364,43 @@ function newAgentId() { return `agent_${crypto.randomUUID()}`; } +/** + * Move coworkers that predate the endpoint-less create fix onto the local run loop. + * + * Before this repair, choosing "Built in" on a deployment with a managed Bot wrote a `remote_ag_ui` + * row whose configuration was only the managed endpoint. That row could answer, but the handoff tool + * is minted only inside this deployment's run loop, so it could never hand work on. The role + * description is the instruction the create route had passed to `systemPrompt`; it is therefore the + * text the repaired `built_in` row must run on. + * + * The managed URL is configuration rather than a database fact, so this is an idempotent startup + * repair instead of a static migration. Rows with auth are skipped: an address a person supplied is + * not the same thing even when it happens to equal this deployment's address. + */ +export async function repairBuiltInCoworkers( + database: Database, + managedAgentAgUiUrl: URL | undefined, +): Promise { + if (!managedAgentAgUiUrl) return 0; + + const repaired = await database.execute(sql` + UPDATE "agents" AS a + SET + "type" = 'built_in', + "configuration" = jsonb_build_object('systemPrompt', p."role_description"), + "updated_at" = now() + FROM "agent_profiles" AS p + WHERE p."agent_id" = a."id" + AND p."deleted_at" IS NULL + AND a."type" = 'remote_ag_ui' + AND a."configuration"->>'endpoint' = ${managedAgentAgUiUrl.toString()} + AND a."configuration"->'auth' IS NULL + RETURNING a."id" + `); + + return repaired.length; +} + /** * Which agent a token belongs to. * @@ -445,7 +482,7 @@ export function createAgentProfileStore( const id = newAgentId(); const endpoint = input.endpoint ? { endpoint: input.endpoint } - : managedConfiguration; + : undefined; const systemPrompt = input.systemPrompt?.trim(); if (endpoint) { await transaction.insert(agents).values({ @@ -490,6 +527,19 @@ export function createAgentProfileStore( type: "built_in", configuration: { systemPrompt }, }); + } else if (managedConfiguration) { + /* + * Store calls that do not carry a prompt keep the old managed-endpoint shape. The create + * route always supplies the role description on the endpoint-less path, so a coworker a + * person makes as "Built in" never reaches here; direct callers that genuinely mean the + * managed Bot keep the address. + */ + await transaction.insert(agents).values({ + id, + name: input.name, + type: "remote_ag_ui", + configuration: managedConfiguration, + }); } else { /* * No address, no Bot in the box, and no instruction to run on. There is nothing to create: diff --git a/server/src/agents/routes.ts b/server/src/agents/routes.ts index c5009c020..b4db07a75 100644 --- a/server/src/agents/routes.ts +++ b/server/src/agents/routes.ts @@ -279,22 +279,23 @@ export function createAgentRoutes( */ builtInAvailable = false, /** - * The managed Bot's own address, so a coworker created without an endpoint can be told apart. + * The managed Bot's own address, so legacy endpoint-based built-in coworkers can be told apart. * - * Creation bakes this address into the coworker's stored configuration, and afterwards nothing in - * the row says whether a person supplied it. The difference matters to exactly one screen: a - * coworker running here calls tools back with the deployment's own credential and needs no setup, - * while one a person hosts needs a callback token put into their process. Without this flag the - * dialog nagged built-in coworkers about a credential they never needed. + * New endpoint-less coworkers are stored as `built_in` and have no endpoint. Rows made before that + * fix still point at this address until the startup repair reaches them. The difference matters to + * exactly one screen: a coworker running here calls tools back with the deployment's own credential + * and needs no setup, while one a person hosts needs a callback token put into their process. */ managedEndpoint?: string, ) { /** The dto with the one fact only this closure knows: whether the coworker runs on our own Bot. */ const dto = (actor: AgentActor, agent: AgentProfile) => ({ ...agentDto(actor, agent), - // A string comparison on purpose: two absent values must not read as "runs on our Bot". + // `null` is the endpoint-less built-in shape; the comparison keeps legacy managed rows marked too. builtIn: - typeof agent.endpoint === "string" && agent.endpoint === managedEndpoint, + agent.endpoint === null || + (typeof agent.endpoint === "string" && + agent.endpoint === managedEndpoint), }); const routes = new Hono<{ Variables: AppVariables }>(); diff --git a/server/src/app.ts b/server/src/app.ts index f5b645225..73cfbbf34 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -1295,8 +1295,8 @@ export function createApp( // Whether "built-in" is a kind of coworker this deployment can actually make: the create // path falls back to the managed Bot's endpoint, so without one it can only refuse. config.managedAgent?.endpoint !== undefined, - // The managed Bot's address, so a coworker created without an endpoint — which creation - // stores as running at this address — can be told apart from one a person hosts. + // The managed Bot's address, so a legacy coworker stored before endpoint-less creates became + // `built_in` can still be told apart from one a person hosts. config.managedAgent?.endpoint?.toString(), ), ); diff --git a/server/src/index.ts b/server/src/index.ts index e40751b76..aed42b748 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -42,7 +42,10 @@ import { createBotLifecycleStore, } from "./agents/lifecycle"; import { createBotReset } from "./agents/lifecycle-reset"; -import { createAgentProfileStore } from "./agents/profile-store"; +import { + createAgentProfileStore, + repairBuiltInCoworkers, +} from "./agents/profile-store"; import type { AgentActor } from "./agents/profile-types"; import { createRuntimeAgentLoader } from "./agents/runtime-agents"; import { @@ -339,6 +342,7 @@ const agentProfileStore = createAgentProfileStore( config.managedAgent?.endpoint, agentVault, ); +await repairBuiltInCoworkers(database, config.managedAgent?.endpoint); // Read here rather than beside the synchronise below, because the package names the deployment and // the channel store needs that name before it can mint a thread id. const tenantPackage = await loadTenantPackage(config.tenantPackageDirectory); diff --git a/server/tests/agent-handoff-endtoend.integration.test.ts b/server/tests/agent-handoff-endtoend.integration.test.ts index 863ee7498..c5b2a1186 100644 --- a/server/tests/agent-handoff-endtoend.integration.test.ts +++ b/server/tests/agent-handoff-endtoend.integration.test.ts @@ -1,6 +1,6 @@ import { afterAll, beforeEach, describe, expect, test } from "bun:test"; import { randomUUID } from "node:crypto"; -import { and, eq, like } from "drizzle-orm"; +import { and, eq, like, or, sql } from "drizzle-orm"; import { createHandoffDesk } from "../src/agents/handoff"; import { createHandoffRunner, @@ -15,6 +15,7 @@ import { agents, auditEvents, pluginGrants, + users, workItems, } from "../src/db/schema"; import { createWorkQueue } from "../src/work/queue"; @@ -42,7 +43,12 @@ const RUN = `e2e-run-${suite}`; const queue = createWorkQueue(database); const auditStore = createAuditStore(database); -const profiles = createAgentProfileStore(database); +const profiles = createAgentProfileStore( + database, + new URL("https://managed.example.test/ag-ui"), +); +const createdAgentIds = new Set(); +const createdUserIds = new Set(); const desk = createHandoffDesk({ queue, @@ -67,12 +73,24 @@ const desk = createHandoffDesk({ }); async function clean() { - await database.delete(workItems).where(like(workItems.key, `${RUN}%`)); - for (const id of [ASKER, TARGET]) { + await database + .delete(workItems) + .where( + or( + like(workItems.key, `${RUN}%`), + sql`${workItems.payload}->>'runId' = ${RUN}`, + ), + ); + for (const id of [ASKER, TARGET, ...createdAgentIds]) { await database.delete(pluginGrants).where(eq(pluginGrants.agentId, id)); await database.delete(agentProfiles).where(eq(agentProfiles.agentId, id)); await database.delete(agents).where(eq(agents.id, id)); } + createdAgentIds.clear(); + for (const id of createdUserIds) { + await database.delete(users).where(eq(users.id, id)); + } + createdUserIds.clear(); } beforeEach(async () => { @@ -109,6 +127,80 @@ afterAll(async () => { }); describe("a hop, from the tool call to the delivery", () => { + test("a coworker created as Built in can hand work on", async () => { + const ownerId = `e2e-built-in-owner-${suite}`; + await database.insert(users).values({ + id: ownerId, + email: `${ownerId}@example.test`, + name: "Built In Owner", + }); + createdUserIds.add(ownerId); + + const source = await profiles.create( + { id: ownerId, role: "user" }, + { + name: "Built In Coordinator", + title: "Coordinator", + roleDescription: "Coordinate work without inventing an endpoint.", + visibility: "private", + systemPrompt: "Coordinate work without inventing an endpoint.", + }, + ); + createdAgentIds.add(source.id); + + const [stored] = await database + .select({ type: agents.type, configuration: agents.configuration }) + .from(agents) + .where(eq(agents.id, source.id)); + expect(stored?.type).toBe("built_in"); + expect(stored?.configuration).toEqual({ + systemPrompt: "Coordinate work without inventing an endpoint.", + }); + + await database.insert(pluginGrants).values({ + kind: "bot", + ref: TARGET, + agentId: source.id, + grantedBy: ownerId, + }); + + const tool = handoffTool({ + desk, + from: { + botId: source.id, + actorId: ownerId, + runId: RUN, + threadId: `thread-${suite}`, + depth: 0, + }, + hasSomebodyToAsk: true, + maxDepth: 2, + }); + const said = await tool?.execute({ + bot: "Target", + task: "confirm the handoff path", + }); + expect(said).toContain("Target"); + + const [queued] = await database + .select({ kind: workItems.kind, payload: workItems.payload }) + .from(workItems) + .where( + and( + eq(workItems.kind, "bot.message"), + sql`${workItems.payload}->>'fromBotId' = ${source.id}`, + ), + ); + expect(queued).toMatchObject({ + kind: "bot.message", + payload: { + fromBotId: source.id, + toBotId: TARGET, + actorId: ownerId, + }, + }); + }); + test("what one Bot asked for is what the other is shown", async () => { const tool = handoffTool({ desk, diff --git a/server/tests/agent-profile-store.integration.test.ts b/server/tests/agent-profile-store.integration.test.ts index 1511ac061..47371210d 100644 --- a/server/tests/agent-profile-store.integration.test.ts +++ b/server/tests/agent-profile-store.integration.test.ts @@ -9,6 +9,7 @@ import { createAgentProfileStore, ManagedAgentUnavailableError, ProtectedAgentError, + repairBuiltInCoworkers, } from "../src/agents/profile-store"; import type { AgentActor, @@ -292,6 +293,67 @@ describe("agent profile store integration", () => { expect(row.configuration.endpoint).toBeUndefined(); }); + test("creates a built-in coworker on a deployment that also has a managed Bot", async () => { + const owner = await createUser(); + + const created = await store.create(owner, { + name: "Runs Here Too", + title: "Everyday Work", + roleDescription: "Use the deployment's own run loop.", + visibility: "private", + systemPrompt: "Use the deployment's own run loop.", + }); + createdAgentIds.push(created.id); + + const row = await agentRow(created.id); + expect(row.type).toBe("built_in"); + expect(row.configuration.systemPrompt).toBe( + "Use the deployment's own run loop.", + ); + // The managed endpoint belongs to the deployment, not to this coworker's identity. + expect(row.configuration.endpoint).toBeUndefined(); + }); + + test("repairs existing managed-endpoint coworkers into built-in rows once", async () => { + const owner = await createUser(); + const legacy = await createProfileFixture({ + owner, + visibility: "private", + roleDescription: "Run on the role description that was already saved.", + configuration: { endpoint: managedAgentAgUiUrl.toString() }, + }); + const authenticated = await createProfileFixture({ + owner, + visibility: "private", + configuration: { + endpoint: managedAgentAgUiUrl.toString(), + auth: { header: "Authorization", credentialId: "external-credential" }, + }, + }); + + expect( + await repairBuiltInCoworkers(database, managedAgentAgUiUrl), + ).toBeGreaterThanOrEqual(1); + const repaired = await agentRow(legacy.agentId); + expect(repaired).toEqual({ + type: "built_in", + configuration: { + systemPrompt: "Run on the role description that was already saved.", + }, + }); + // A row carrying its own credential is a deployment's choice, not the old built-in shape. + expect(await agentRow(authenticated.agentId)).toEqual({ + type: "remote_ag_ui", + configuration: { + endpoint: managedAgentAgUiUrl.toString(), + auth: { header: "Authorization", credentialId: "external-credential" }, + }, + }); + + await repairBuiltInCoworkers(database, managedAgentAgUiUrl); + expect(await agentRow(legacy.agentId)).toEqual(repaired); + }); + test("an edit moves the instruction such a coworker actually runs on", async () => { const owner = await createUser(); const withoutManaged = createAgentProfileStore(database, undefined);