diff --git a/apps/web/src/instant-agent-create.test.ts b/apps/web/src/instant-agent-create.test.ts index d6c26357..6c6fc9ac 100644 --- a/apps/web/src/instant-agent-create.test.ts +++ b/apps/web/src/instant-agent-create.test.ts @@ -1,4 +1,8 @@ import { afterEach, describe, expect, test } from "bun:test"; +import { + CODE_REVIEW_TEMPLATE, + serializeWorkbenchTemplateManifest, +} from "@corbits/workflow-catalog"; import { createWorkbenchFromTemplate, @@ -79,5 +83,96 @@ describe("createWorkbenchFromTemplate (CL-6387)", () => { expect(createCalls).toHaveLength(2); expect(navigated).toEqual(["/w/chan-1", "/w/chan-2"]); expect(navigated[0]).not.toBe(navigated[1]); + + // Blank ("Just start talking") has no template to name the bench + // after, so it keeps the generic title rather than something + // invented. + const body = JSON.parse(String(createCalls[0]?.init?.body)); + expect(body.name).toBe(NEW_WORKBENCH_TITLE); + }); + + // CL-6387 follow-up: picking a named template threw its own name away + // and left the reviewer roster its greeting promises out of the room + // (every bench looked like every other "New Workbench", and Myra's + // "Three reviewers read every pull request" greeting described a team + // that wasn't there — see `createWorkbenchFromTemplate`'s own doc). + test("picking the code-review template names the bench after it and invites the whole reviewer roster", async () => { + const navigated: string[] = []; + let nextReviewerId = 0; + const calls = stubFetch((path) => { + if (path.includes("/workflows/definitions")) { + return json({ data: [assistantDefinitionWire], nextCursor: null }); + } + if (path.endsWith("/library/templates/code-review")) { + return json({ + id: "code-review", + content: serializeWorkbenchTemplateManifest(CODE_REVIEW_TEMPLATE), + }); + } + if (path.endsWith("/chat/workbenches")) { + return json({ + id: "chan-1", + title: "Code review", + kind: "chat", + pinned: false, + participants: [], + }); + } + if (path.endsWith("/template-blocks/code-review/deploy")) { + return json({ id: "def-code-review-block", created: true }); + } + if (path.endsWith("/agent-definitions")) { + nextReviewerId += 1; + return json({ + ...assistantDefinitionWire, + id: `def-reviewer-${nextReviewerId}`, + }); + } + if (path.endsWith("/chat/workbenches/chan-1/invite")) { + return json({ address: "agent:invited", definitionId: "def-reviewer" }); + } + if (path.endsWith("/chat/workbenches/chan-1/settings")) { + return json({ + id: "chan-1", + title: "Code review", + kind: "chat", + pinned: false, + participants: [], + settings: {}, + contextWindow: { value: 0, source: "inherit" }, + }); + } + throw new Error(`unexpected fetch: ${path}`); + }); + + await createWorkbenchFromTemplate("tnt_1", "code-review", (to) => + navigated.push(to), + ); + + const createCall = calls.find((call) => + call.path.endsWith("/chat/workbenches"), + ); + const createBody = JSON.parse(String(createCall?.init?.body)); + expect(createBody.name).toBe(CODE_REVIEW_TEMPLATE.title); + + const createAgentCalls = calls.filter((call) => + call.path.endsWith("/agent-definitions"), + ); + const reviewerCount = CODE_REVIEW_TEMPLATE.participants.filter( + (participant) => participant.handle !== "myra", + ).length; + expect(createAgentCalls).toHaveLength(reviewerCount); + + const inviteCalls = calls.filter((call) => + call.path.endsWith("/chat/workbenches/chan-1/invite"), + ); + const invitedIds = inviteCalls.map( + (call) => JSON.parse(String(call.init?.body)).definitionId, + ); + const createdIds = createAgentCalls.map( + (_, index) => `def-reviewer-${index + 1}`, + ); + expect(invitedIds.sort()).toEqual(createdIds.sort()); + expect(navigated).toEqual(["/w/chan-1"]); }); }); diff --git a/apps/web/src/instant-agent-create.ts b/apps/web/src/instant-agent-create.ts index 4f7e0b41..393ff9de 100644 --- a/apps/web/src/instant-agent-create.ts +++ b/apps/web/src/instant-agent-create.ts @@ -17,6 +17,7 @@ import { getLogger } from "@corbits/client-log"; import { createWorkbench, getConnectGithubState, + inviteAgent, patchWorkbenchSettings, startReviewingGithubRepos, type ConnectGithubRepo, @@ -68,21 +69,22 @@ export type PickGithubRepos = (args: { /** * The template picker's "Create workbench" action (CL-6344): mints a - * fresh "New Workbench" chat against the account's default setup template - * (the seeded `assistant`/Myra definition), passing the picked row's id - * through as `templateId` so the room opens with that template's own intro - * (`packages/chat/src/routes.ts`'s `POST /workbenches` resolves it into - * the canned greeting). When the id names a real manifest - * (`workbenchTemplate`), this also creates its participant agent - * definitions and records its required connections as pending — see - * `instantiateWorkbenchTemplate`'s own doc for exactly what that does - * and does not do yet (inviting the reviewers into the room, and the - * GitHub connect card itself, are the next slice). A template id with - * no manifest yet (`blank`, "Just start talking") mints a plain - * untagged chat, exactly like before templates existed. When - * `pickGithubRepos` is supplied and GitHub is already connected for this - * tenant, this also drives CL-6386's "select on new-workbench" step — - * see `PickGithubRepos`'s own doc. + * fresh chat, named after the picked template, against the account's + * default setup template (the seeded `assistant`/Myra definition), + * passing the picked row's id through as `templateId` so the room opens + * with that template's own intro (`packages/chat/src/routes.ts`'s + * `POST /workbenches` resolves it into the canned greeting). When the id + * names a real manifest (`workbenchTemplate`), this also creates its + * participant agent definitions, invites each into the room so the + * roster the greeting promises is the roster actually there (see + * `instantiateWorkbenchTemplate`'s own doc), and records its required + * connections as pending. A template id with no manifest yet (`blank`, + * "Just start talking") mints a plain untagged chat under the generic + * `NEW_WORKBENCH_TITLE`, exactly like before templates existed — there + * is no better name to give it. When `pickGithubRepos` is supplied and + * GitHub is already connected for this tenant, this also drives + * CL-6386's "select on new-workbench" step — see `PickGithubRepos`'s + * own doc. */ export async function createWorkbenchFromTemplate( tenantId: string, @@ -133,7 +135,7 @@ export async function createWorkbenchFromTemplate( const workbench = await createWorkbench(tenantId, { kind: "chat", definitionId: setupTemplate.id, - name: NEW_WORKBENCH_TITLE, + name: manifest?.title ?? NEW_WORKBENCH_TITLE, ...(manifest !== undefined ? { templatePromise: manifest.promise } : {}), ...(requiresGithub && !githubAlreadyConnected ? { connectGithubRequiredFor: manifest?.title ?? "" } @@ -158,7 +160,10 @@ export async function createWorkbenchFromTemplate( const result = await instantiateWorkbenchTemplate(manifest, { async listAgentHandles() { const current = await listAgentDefinitions(tenantId); - return current.map((definition) => definition.name); + return current.map((definition) => ({ + handle: definition.name, + id: definition.id, + })); }, async createParticipantAgent(request) { const created = await createAgentDefinition(tenantId, request); @@ -167,6 +172,9 @@ export async function createWorkbenchFromTemplate( async deployBlockWorkflow(block) { return deployWorkbenchTemplateBlock(tenantId, block.assetName); }, + async inviteParticipantAgent(id) { + await inviteAgent(tenantId, workbench.id, id); + }, async recordPendingConnections(pendingConnections) { await patchWorkbenchSettings( tenantId, diff --git a/apps/web/test/new-workbench-picker.test.tsx b/apps/web/test/new-workbench-picker.test.tsx index 06a5d254..b5887bfb 100644 --- a/apps/web/test/new-workbench-picker.test.tsx +++ b/apps/web/test/new-workbench-picker.test.tsx @@ -292,6 +292,18 @@ describe("NewWorkbenchPickerRoute", () => { skills: [], }); } + if ( + path.endsWith("/chat/workbenches/chan_new/invite") && + init?.method === "POST" + ) { + const body = JSON.parse(String(init.body)) as { + definitionId: string; + }; + return json({ + address: `${body.definitionId}@chan_new`, + definitionId: body.definitionId, + }); + } if (path.endsWith("/chat/workbenches/chan_new/settings")) { return json({ id: "chan_new", @@ -414,6 +426,18 @@ describe("NewWorkbenchPickerRoute", () => { skills: [], }); } + if ( + path.endsWith("/chat/workbenches/chan_new/invite") && + init?.method === "POST" + ) { + const body = JSON.parse(String(init.body)) as { + definitionId: string; + }; + return json({ + address: `${body.definitionId}@chan_new`, + definitionId: body.definitionId, + }); + } if (path.endsWith("/chat/workbenches/chan_new/settings")) { return json({ id: "chan_new", diff --git a/packages/evals/src/targets/real-target.ts b/packages/evals/src/targets/real-target.ts index cbe23c24..8fe88922 100644 --- a/packages/evals/src/targets/real-target.ts +++ b/packages/evals/src/targets/real-target.ts @@ -734,8 +734,8 @@ export async function bootMyraTarget( res.data, "data", "list agent definitions", - ) as { name: string }[]; - return rows.map((row) => row.name); + ) as { name: string; id: string }[]; + return rows.map((row) => ({ handle: row.name, id: row.id })); }, async createParticipantAgent(request) { const res = await api( @@ -755,6 +755,21 @@ export async function bootMyraTarget( id: stringField(res.data, "id", `created "${request.handle}"`), }; }, + async inviteParticipantAgent(id) { + const res = await api( + hub.baseUrl, + "POST", + `/api/tenants/${seeded.tenantId}/chat/workbenches/${workbenchId}/invite`, + { definitionId: id }, + cookies, + ); + if (res.status !== 200 && res.status !== 201) { + throw new Error( + `invite participant agent "${id}" returned ` + + `${String(res.status)}: ${JSON.stringify(res.data)}`, + ); + } + }, async deployBlockWorkflow(block) { const res = await api( hub.baseUrl, diff --git a/packages/workflow-catalog/src/instantiate.ts b/packages/workflow-catalog/src/instantiate.ts index 600ef475..68d6d8ca 100644 --- a/packages/workflow-catalog/src/instantiate.ts +++ b/packages/workflow-catalog/src/instantiate.ts @@ -45,11 +45,17 @@ export type ParticipantAgentRequest = CodeReviewAgentRequest & { }; export interface WorkbenchTemplateInstantiationPorts { - /** Every agent definition handle already deployed in the bench — - * the idempotency check so re-running instantiation (a retried - * create, a second workbench from the same template) never double- - * creates a reviewer. */ - listAgentHandles(): Promise; + /** Every agent definition already deployed in the bench, as + * `{handle, id}` pairs. Doubles as the idempotency check (re-running + * instantiation — a retried create, a second workbench from the same + * template — never double-creates a reviewer) and as the id source + * for `inviteParticipantAgent` below: an agent definition the tenant + * already has is not yet a participant of a freshly minted room, so + * its id still has to reach the invite call even when this function + * skips creating it. */ + listAgentHandles(): Promise< + readonly { readonly handle: string; readonly id: string }[] + >; /** The agent-directory create path (`POST /agent-definitions`), or a * fake of it in tests. */ createParticipantAgent( @@ -65,6 +71,14 @@ export interface WorkbenchTemplateInstantiationPorts { deployBlockWorkflow( block: WorkbenchTemplateBlock, ): Promise<{ readonly created: boolean }>; + /** Adds one participant's agent definition to the newly created + * workbench's room (`POST /workbenches/:id/invite` — + * `@corbits/chat-ui`'s `inviteAgent`), or a fake of it in tests. This + * is what makes a template's roster actually present in the room + * rather than merely registered in the agent directory. Never called + * for Myra: she joins the room at workbench creation as its own + * `definitionId`. */ + inviteParticipantAgent(id: string): Promise; /** Persists the room's still-needed connections — the workbench * settings `template/pendingConnections` key today; see * `apps/web/src/instant-agent-create.ts`. */ @@ -76,6 +90,11 @@ export interface WorkbenchTemplateInstantiationPorts { export interface WorkbenchTemplateInstantiationResult { readonly createdHandles: readonly string[]; readonly skippedHandles: readonly string[]; + /** Every non-Myra participant handle actually added to the room — + * `createdHandles` and `skippedHandles` combined, in manifest order. + * A caller proving the roster a template's greeting promises is + * really present checks this, not just that the definitions exist. */ + readonly invitedHandles: readonly string[]; /** Block workflows `deployBlockWorkflow` actually deployed on this * run, by asset name; a block the tenant already carried lands in * `skippedBlockAssetNames` instead. */ @@ -119,7 +138,9 @@ export async function instantiateWorkbenchTemplate( manifest: WorkbenchTemplateManifest, ports: WorkbenchTemplateInstantiationPorts, ): Promise { - const existingHandles = new Set(await ports.listAgentHandles()); + const existingIdsByHandle = new Map( + (await ports.listAgentHandles()).map((agent) => [agent.handle, agent.id]), + ); const requestsByHandle = new Map( [ ...codeReviewAgentRequests(), @@ -144,21 +165,28 @@ export async function instantiateWorkbenchTemplate( const createdHandles: string[] = []; const skippedHandles: string[] = []; + const invitedHandles: string[] = []; for (const participant of manifest.participants) { if (participant.handle === "myra") continue; - if (existingHandles.has(participant.handle)) { + const existingId = existingIdsByHandle.get(participant.handle); + let participantId: string; + if (existingId !== undefined) { skippedHandles.push(participant.handle); - continue; - } - const request = requestsByHandle.get(participant.handle); - if (request === undefined) { - throw new Error( - `workbench template "${manifest.id}" participant "${participant.handle}" ` + - "has no known create-agent request to instantiate it from", - ); + participantId = existingId; + } else { + const request = requestsByHandle.get(participant.handle); + if (request === undefined) { + throw new Error( + `workbench template "${manifest.id}" participant "${participant.handle}" ` + + "has no known create-agent request to instantiate it from", + ); + } + const created = await ports.createParticipantAgent(request); + createdHandles.push(participant.handle); + participantId = created.id; } - await ports.createParticipantAgent(request); - createdHandles.push(participant.handle); + await ports.inviteParticipantAgent(participantId); + invitedHandles.push(participant.handle); } await ports.recordPendingConnections(manifest.requiredConnections); @@ -166,6 +194,7 @@ export async function instantiateWorkbenchTemplate( return { createdHandles, skippedHandles, + invitedHandles, deployedBlockAssetNames, skippedBlockAssetNames, pendingConnections: manifest.requiredConnections, diff --git a/packages/workflow-catalog/test/instantiate.test.ts b/packages/workflow-catalog/test/instantiate.test.ts index c4e4c3f7..e1cf2128 100644 --- a/packages/workflow-catalog/test/instantiate.test.ts +++ b/packages/workflow-catalog/test/instantiate.test.ts @@ -16,18 +16,21 @@ function fakePorts( alreadyDeployedBlocks: readonly string[] = [], ): WorkbenchTemplateInstantiationPorts & { readonly created: string[]; + readonly invited: string[]; readonly recordedConnections: (readonly string[])[]; readonly deployedBlocks: string[]; } { const created: string[] = []; + const invited: string[] = []; const recordedConnections: (readonly string[])[] = []; const deployedBlocks: string[] = []; return { created, + invited, recordedConnections, deployedBlocks, async listAgentHandles() { - return existingHandles; + return existingHandles.map((handle) => ({ handle, id: `def-${handle}` })); }, async createParticipantAgent(request) { created.push(request.handle); @@ -37,6 +40,9 @@ function fakePorts( deployedBlocks.push(block.assetName); return { created: !alreadyDeployedBlocks.includes(block.assetName) }; }, + async inviteParticipantAgent(id) { + invited.push(id); + }, async recordPendingConnections(pendingConnections) { recordedConnections.push(pendingConnections); }, @@ -83,6 +89,36 @@ test("instantiating the code-review template skips a reviewer that already exist expect(ports.created).not.toContain("architecture-reviewer"); }); +// A reviewer whose agent-directory definition already exists (a second +// workbench from the same template) still has to become a participant +// of THIS new room — an existing definition is not an existing +// invitation, so skipping the create must never skip the invite too. +test("instantiating the code-review template invites every reviewer into the room, created or skipped alike", async () => { + const ports = fakePorts(["architecture-reviewer"]); + const result = await instantiateWorkbenchTemplate( + CODE_REVIEW_TEMPLATE, + ports, + ); + expect(result.invitedHandles).toEqual( + CODE_REVIEW_REVIEWERS.map((reviewer) => reviewer.handle), + ); + expect(ports.invited).toEqual( + expect.arrayContaining([ + "def-architecture-reviewer", + "def-correctness-reviewer", + "def-release-risk-reviewer", + ]), + ); + expect(ports.invited).toHaveLength(3); +}); + +test("instantiating the code-review template never invites Myra — she is already the room's own agent", async () => { + const ports = fakePorts(); + await instantiateWorkbenchTemplate(CODE_REVIEW_TEMPLATE, ports); + expect(ports.invited).not.toContain("def-myra"); + expect(ports.invited).toHaveLength(CODE_REVIEW_REVIEWERS.length); +}); + test("instantiating the code-review template names an honest pending note for its not-yet-scoped webhook trigger", async () => { const ports = fakePorts(); const result = await instantiateWorkbenchTemplate( @@ -165,6 +201,9 @@ test("Scout's create request carries its tool package pins", async () => { async deployBlockWorkflow() { return { created: true }; }, + async inviteParticipantAgent() { + /* noop */ + }, async recordPendingConnections() { /* noop */ },