diff --git a/PRODUCT.md b/PRODUCT.md index cfefc00e2..2cb746677 100644 --- a/PRODUCT.md +++ b/PRODUCT.md @@ -36,15 +36,15 @@ column at a time: bench, flat, most-recently-active first. There is no separate "channels" vs. "chats" grouping the sidebar exposes to a person — every row is a workbench. -- **"+ New Workbench" always creates.** It opens `/new` — the shipped - prompt-primary picker — never a picker of existing things to join. - Starting a new workbench is the one way in, whether the result is a - one-on-one conversation with an agent or a group conversation - with people and agents together. -- **Agents are templates.** Starting a new agent conversation means picking - an agent definition (a named, reusable capability) as the starting point - — the same definition can be launched into any number of separate - conversations, each with its own history and its own tenant. +- **"+ New Workbench" always creates a room.** It opens `/new` — the shipped + prompt-primary picker — never a picker of existing things to join, and + never reopens an agent's one conversation. Starting a new workbench mints + a fresh room; opening an agent (from Agents, Talk to Myra, and so on) + find-or-reopens that agent's existing conversation. +- **Agents are templates.** An agent definition is a named, reusable + capability. Opening an agent always find-or-reopens its one conversation + for this bench; "+" creates a separate room and invites participants into + it — including Myra — rather than minting another agent DM. - The active workbench occupies the main column; a contextual panel beside it carries account-wide surfaces (approvals, recent activity) that stay visible regardless of which workbench is open. diff --git a/apps/web/src/agent-chat-launch.test.ts b/apps/web/src/agent-chat-launch.test.ts index 96f801af0..f55c81e98 100644 --- a/apps/web/src/agent-chat-launch.test.ts +++ b/apps/web/src/agent-chat-launch.test.ts @@ -28,7 +28,7 @@ describe("launchAgentChat", () => { headers: { "content-type": "application/json" }, }); - test("creates a chat for the given definitionId with no reuseExisting flag, and navigates to it", async () => { + test("opens a chat for the given definitionId and navigates to it", async () => { const navigated: string[] = []; const calls = stubFetch((path) => { if (path.endsWith("/chat/workbenches")) { diff --git a/apps/web/src/agent-chat-launch.ts b/apps/web/src/agent-chat-launch.ts index cafb0ebef..d54e06965 100644 --- a/apps/web/src/agent-chat-launch.ts +++ b/apps/web/src/agent-chat-launch.ts @@ -1,13 +1,12 @@ // The one path from "an agent's definitionId" to "the person is in a -// fresh chat with it" — the same `POST /workbenches` call this app's every -// create path uses. `CreateAgentPanel`'s Settings → Agents entry point -// calls this on success so an explicitly-defined new agent never ends -// nowhere, and `instant-agent-create.ts` — THE one creation verb -// (CL-6138) — calls it against the account's default setup template. -// Always creates (CL-6089) — never the `reuseExisting` land-hop path, -// which is `default-agent-workbench.ts`'s own call, not this one. +// chat with it" — `openAgentConversation` find-or-reopens the agent's +// one conversation (CL-6981). `CreateAgentPanel`'s Settings → Agents +// entry point calls this on success so an explicitly-defined new agent +// never ends nowhere, and `instant-agent-create.ts` — THE one creation +// verb (CL-6138) — calls against the account's default setup template +// through `createWorkbench` with template fields, not this hop. -import { createWorkbench } from "@corbits/chat-ui"; +import { createWorkbench, openAgentConversation } from "@corbits/chat-ui"; import { workbenchPath } from "./workbench-path"; @@ -17,10 +16,13 @@ export async function launchAgentChat( navigate: (to: string) => void, name?: string, ): Promise { - const workbench = await createWorkbench(tenantId, { - kind: "chat", - definitionId, - ...(name !== undefined ? { name } : {}), - }); + const workbench = + name === undefined + ? await openAgentConversation(tenantId, definitionId) + : await createWorkbench(tenantId, { + kind: "chat", + definitionId, + name, + }); navigate(workbenchPath(workbench.id)); } diff --git a/apps/web/src/agent-dm-launch.test.ts b/apps/web/src/agent-dm-launch.test.ts index ccab573f0..0c2ccf359 100644 --- a/apps/web/src/agent-dm-launch.test.ts +++ b/apps/web/src/agent-dm-launch.test.ts @@ -28,7 +28,7 @@ describe("openAgentDmChat", () => { headers: { "content-type": "application/json" }, }); - test("opens the agent's DM with reuseExisting and navigates to it", async () => { + test("opens the agent's conversation and navigates to it", async () => { const navigated: string[] = []; const calls = stubFetch((path) => { if (path.endsWith("/chat/workbenches")) { @@ -52,7 +52,6 @@ describe("openAgentDmChat", () => { expect(JSON.parse(String(call?.init?.body))).toEqual({ kind: "chat", definitionId: "wfd_outreach", - reuseExisting: true, }); expect(navigated).toEqual(["/w/chan-dm-1"]); }); diff --git a/apps/web/src/agent-dm-launch.ts b/apps/web/src/agent-dm-launch.ts index 7bc9fcec6..65656346c 100644 --- a/apps/web/src/agent-dm-launch.ts +++ b/apps/web/src/agent-dm-launch.ts @@ -1,13 +1,12 @@ // The one path from an agent definition's id to "the person is in their // direct chat with it" (CL-6253) — the sidebar's agent rows are the one -// caller. Mirrors `agent-chat-launch.ts`'s shape exactly, but through -// `openAgentDm` (`kind: "chat"`, `reuseExisting: true`) rather than -// `createWorkbench` directly: the first click mints the DM, every later -// click finds the same workbench by `chat/definitionId` +// caller. Mirrors `agent-chat-launch.ts`'s shape exactly, through +// `openAgentConversation`: the first click mints the conversation, every +// later click finds the same workbench by `chat/definitionId` // (`findExistingAgentChat` in `packages/chat/src/routes.ts`) instead of // spawning a new one each time. -import { openAgentDm } from "@corbits/chat-ui"; +import { openAgentConversation } from "@corbits/chat-ui"; import { workbenchPath } from "./workbench-path"; @@ -16,6 +15,6 @@ export async function openAgentDmChat( definitionId: string, navigate: (to: string) => void, ): Promise { - const workbench = await openAgentDm(tenantId, definitionId); + const workbench = await openAgentConversation(tenantId, definitionId); navigate(workbenchPath(workbench.id)); } diff --git a/apps/web/src/instant-agent-create.test.ts b/apps/web/src/instant-agent-create.test.ts index a6616da04..de63c984b 100644 --- a/apps/web/src/instant-agent-create.test.ts +++ b/apps/web/src/instant-agent-create.test.ts @@ -56,7 +56,8 @@ describe("createWorkbenchFromTemplate (CL-6387)", () => { // The picker's "Create workbench" is clickable more than once per // session (a second visit, a second row) — each click must mint its // own, genuinely distinct workbench, never reopen or alias the last - // one it created. + // one it created. CL-6981: blank must POST kind=workbench without + // Myra's definitionId, or a second "+" would find-or-reopen her DM. test("picking the same row twice in a row mints two distinct workbenches, not one reused", async () => { let nextId = 0; const navigated: string[] = []; @@ -69,11 +70,17 @@ describe("createWorkbenchFromTemplate (CL-6387)", () => { return json({ id: `chan-${nextId}`, title: NEW_WORKBENCH_TITLE, - kind: "chat", + kind: "workbench", pinned: false, participants: [], }); } + if (/\/chat\/workbenches\/chan-\d+\/invite$/.test(path)) { + return json({ + address: "agent:myra@room", + definitionId: "def-assistant", + }); + } throw new Error(`unexpected fetch: ${path}`); }); @@ -99,9 +106,23 @@ describe("createWorkbenchFromTemplate (CL-6387)", () => { // Blank ("Just start talking") has no template to name the bench // after, so it keeps the generic title rather than something - // invented. + // invented. CL-6981: a room mint, never a Myra chat reopen. const body = JSON.parse(String(createCalls[0]?.init?.body)); - expect(body.name).toBe(NEW_WORKBENCH_TITLE); + expect(body).toEqual({ + kind: "workbench", + name: NEW_WORKBENCH_TITLE, + }); + expect(body).not.toHaveProperty("definitionId"); + + const inviteBodies = calls.filter((call) => + /\/chat\/workbenches\/chan-\d+\/invite$/.test(call.path), + ); + expect(inviteBodies).toHaveLength(2); + for (const invite of inviteBodies) { + expect(JSON.parse(String(invite.init?.body))).toEqual({ + definitionId: "def-assistant", + }); + } }); // CL-6387 follow-up: picking a named template threw its own name away @@ -126,7 +147,7 @@ describe("createWorkbenchFromTemplate (CL-6387)", () => { return json({ id: "chan-1", title: "Code review", - kind: "chat", + kind: "workbench", pinned: false, participants: [], }); @@ -148,7 +169,7 @@ describe("createWorkbenchFromTemplate (CL-6387)", () => { return json({ id: "chan-1", title: "Code review", - kind: "chat", + kind: "workbench", pinned: false, participants: [], settings: {}, @@ -176,7 +197,11 @@ describe("createWorkbenchFromTemplate (CL-6387)", () => { call.path.endsWith("/chat/workbenches"), ); const createBody = JSON.parse(String(createCall?.init?.body)); - expect(createBody.name).toBe(CODE_REVIEW_TEMPLATE.title); + expect(createBody).toEqual({ + kind: "workbench", + name: CODE_REVIEW_TEMPLATE.title, + }); + expect(createBody).not.toHaveProperty("definitionId"); const createAgentCalls = calls.filter((call) => call.path.endsWith("/agent-definitions"), @@ -195,7 +220,7 @@ describe("createWorkbenchFromTemplate (CL-6387)", () => { const createdIds = createAgentCalls.map( (_, index) => `def-reviewer-${index + 1}`, ); - expect(invitedIds.sort()).toEqual(createdIds.sort()); + expect(invitedIds.sort()).toEqual(["def-assistant", ...createdIds].sort()); expect(navigated).toEqual(["/w/chan-1"]); // CL-6594: a room this function navigates to must never carry a diff --git a/apps/web/src/instant-agent-create.ts b/apps/web/src/instant-agent-create.ts index 538880f8f..6dcf291a0 100644 --- a/apps/web/src/instant-agent-create.ts +++ b/apps/web/src/instant-agent-create.ts @@ -3,13 +3,11 @@ // (CL-6486, superseding CL-6138's silent auto-mint) — opens the template // picker (`pages/new-workbench-picker.tsx`, CL-6342) and calls // `createWorkbenchFromTemplate` below once a row is chosen. It mints a -// fresh workbench against the account's default setup template (the same -// seeded `assistant` definition backing the home Myra workbench, which -// already opens with the setup greeting: "what do you want me around -// for?"). The conversation itself is what specializes the agent into -// whatever the person wants; the drafting and capability machinery already -// listens for that in-chat, so no definition is drafted or created up -// front here. Explicitly defining a brand-new agent template, with its own +// fresh `kind: "workbench"` room (never `kind: "chat"` + Myra's +// definitionId — that always find-or-reopens the one agent conversation, +// CL-6981), then invites Myra in as a participant. Named templates also +// create their roster and invite each non-Myra agent after the mint. +// Explicitly defining a brand-new agent template, with its own // name/purpose/model/skills chosen up front, stays `CreateAgentPanel`'s job // (Settings → Agents), unchanged. @@ -99,22 +97,22 @@ export type PickGithubRepos = (args: { /** * The template picker's "Create workbench" action (CL-6344): mints a - * 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 + * fresh `kind: "workbench"` room, named after the picked template (or + * `NEW_WORKBENCH_TITLE` for blank), then invites Myra in as a participant. + * Opening an agent via `kind: "chat"` + definitionId always find-or-reopens + * that agent's one conversation (CL-6981), so "+" must never mint that way — + * a second "+" would reopen Myra instead of creating another room. + * + * 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. + * "Just start talking") mints a plain room under the generic + * `NEW_WORKBENCH_TITLE`, then invites Myra — 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. * * `queryClient` invalidates the workbenches list once every template * participant has been invited (CL-6594) — `ChatWorkspace`'s own @@ -170,10 +168,12 @@ export async function createWorkbenchFromTemplate( manifest?.requiredConnections.includes("github") ?? false; // GitHub already connected (established from the Plugins page, CL-6386) - // means this create flow can skip the in-room connect card entirely and - // go straight to repo selection once the workbench exists. Not yet - // connected keeps today's exact behaviour: the in-room card stays the - // just-in-time fallback. + // means this create flow can skip waiting on an in-room connect card + // and go straight to repo selection once the workbench exists. Not yet + // connected keeps the Plugins path as the just-in-time connect step; + // a room mint no longer auto-hosts Myra as `definitionId`, so the + // create-time `connectGithubRequiredFor` block (chat-mint only) is + // not posted here. const githubAlreadyConnected = requiresGithub && pickGithubRepos !== undefined ? (await listPluginsForTenant(tenantId)).some( @@ -182,16 +182,16 @@ export async function createWorkbenchFromTemplate( ) : false; + // A room, not a Myra DM reopen (CL-6981): `kind: "chat"` + definitionId + // always find-or-reopens that agent's one conversation. const workbench = await createWorkbench(tenantId, { - kind: "chat", - definitionId: setupTemplate.id, + kind: "workbench", name: manifest?.title ?? NEW_WORKBENCH_TITLE, - ...(manifest !== undefined ? { templatePromise: manifest.promise } : {}), - ...(requiresGithub && !githubAlreadyConnected - ? { connectGithubRequiredFor: manifest?.title ?? "" } - : {}), }); + // Myra joins as an invited participant — never as the mint identity. + await inviteAgent(tenantId, workbench.id, setupTemplate.id); + if (githubAlreadyConnected && pickGithubRepos !== undefined) { const state = await getConnectGithubState(tenantId, workbench.id); if (state.kind === "connected" && state.repos.length > 0) { @@ -239,11 +239,12 @@ export async function createWorkbenchFromTemplate( for (const todo of result.webhookTriggerTodos) { log.error(todo); } - await queryClient.invalidateQueries({ - queryKey: workbenchesQueryKeyPrefix(tenantId), - }); } + await queryClient.invalidateQueries({ + queryKey: workbenchesQueryKeyPrefix(tenantId), + }); + if (firstMessage !== undefined && firstMessage.trim() !== "") { await sendMessage(tenantId, workbench.id, partsForSend(firstMessage, [])); } diff --git a/apps/web/src/myra-workbench.test.ts b/apps/web/src/myra-workbench.test.ts index c42f4bdeb..e1edb60f7 100644 --- a/apps/web/src/myra-workbench.test.ts +++ b/apps/web/src/myra-workbench.test.ts @@ -159,7 +159,6 @@ describe("ensureMyraWorkbench", () => { kind: "chat", definitionId: "def-assistant", name: "Myra", - reuseExisting: true, }); expect(isMyraWorkbenchId("chat-1")).toBe(true); }); diff --git a/apps/web/test/new-workbench-picker.test.tsx b/apps/web/test/new-workbench-picker.test.tsx index 7d9c00910..18265a37f 100644 --- a/apps/web/test/new-workbench-picker.test.tsx +++ b/apps/web/test/new-workbench-picker.test.tsx @@ -163,8 +163,8 @@ function prefabCards(): HTMLButtonElement[] { } /** The standard fixture for a create that mints against `blank` (no - * manifest, no participants, no settings patch) — shared by every test - * exercising the prompt box's blank-plus-first-message path. */ + * manifest, no participants beyond Myra, no settings patch) — shared by + * every test exercising the prompt box's blank-plus-first-message path. */ function stubBlankCreate( onSendMessage?: (body: { parts: readonly { kind: string }[] }) => void, ): RecordedCall[] { @@ -189,11 +189,21 @@ function stubBlankCreate( return json({ id: "chan_new", title: "New Workbench", - kind: "chat", + kind: "workbench", pinned: false, participants: [], }); } + 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/messages") && init?.method === "POST" @@ -346,9 +356,12 @@ describe("NewWorkbenchPickerRoute", () => { call.path.endsWith("/chat/workbenches") && call.init?.method === "POST", ); expect(JSON.parse(String(createWorkbenchCall?.init?.body))).toMatchObject({ - kind: "chat", - definitionId: "wfd_assistant", + kind: "workbench", + name: "New Workbench", }); + expect( + JSON.parse(String(createWorkbenchCall?.init?.body)), + ).not.toHaveProperty("definitionId"); const sendMessageCall = calls.find((call) => call.path.endsWith("/chat/workbenches/chan_new/messages"), @@ -460,7 +473,7 @@ describe("NewWorkbenchPickerRoute", () => { return json({ id: "chan_new", title: "New Workbench", - kind: "chat", + kind: "workbench", pinned: false, participants: [], }); @@ -501,7 +514,7 @@ describe("NewWorkbenchPickerRoute", () => { return json({ id: "chan_new", title: "New Workbench", - kind: "chat", + kind: "workbench", pinned: false, participants: [], settings: { @@ -543,11 +556,15 @@ describe("NewWorkbenchPickerRoute", () => { call.path.endsWith("/chat/workbenches") && call.init?.method === "POST", ); expect(JSON.parse(String(createWorkbenchCall?.init?.body))).toMatchObject({ - kind: "chat", - definitionId: "wfd_assistant", - templatePromise: - "Three reviewers read every pull request and post what they'd change.", + kind: "workbench", + name: "Code review", }); + expect( + JSON.parse(String(createWorkbenchCall?.init?.body)), + ).not.toHaveProperty("definitionId"); + expect( + JSON.parse(String(createWorkbenchCall?.init?.body)), + ).not.toHaveProperty("templatePromise"); expect(createdAgentHandles).toEqual([ "correctness-reviewer", @@ -595,7 +612,7 @@ describe("NewWorkbenchPickerRoute", () => { return json({ id: "chan_new", title: "New Workbench", - kind: "chat", + kind: "workbench", pinned: false, participants: [], }); @@ -635,7 +652,7 @@ describe("NewWorkbenchPickerRoute", () => { return json({ id: "chan_new", title: "New Workbench", - kind: "chat", + kind: "workbench", pinned: false, participants: [], settings: { diff --git a/packages/chat-ui/src/api.ts b/packages/chat-ui/src/api.ts index a037bf9ab..690055dea 100644 --- a/packages/chat-ui/src/api.ts +++ b/packages/chat-ui/src/api.ts @@ -333,18 +333,15 @@ export function listAllWorkbenches( // counterpart attached at creation. See `packages/chat/src/routes.ts` // `POST /workbenches` for the server side of this union. // -// An agent chat always mints a new workbench (CL-6089) — the agent is a -// template, not a conversation being reopened — unless the caller opts -// into `reuseExisting: true`, reserved for the one deliberate -// find-or-create caller: the home-workbench land-hop -// (`default-agent-workbench.ts`'s `ensure`). +// An agent chat (`kind: "chat"` + `definitionId`) always find-or-reopens +// the one conversation for that agent (CL-6981). Callers that just want +// to open it use `openAgentConversation`. export type CreateWorkbenchInput = | { readonly kind: "workbench"; readonly name: string } | { readonly kind: "chat"; readonly definitionId: string; readonly name?: string; - readonly reuseExisting?: boolean; /** The picked template's own promise line * (`WorkbenchTemplateManifest.promise`, see `@corbits/workflow-catalog`) * — replaces the room's random canned opener with one naming its @@ -854,24 +851,29 @@ export function listVisibleAgentDefinitions( /** * Opens a direct chat with an agent, minting it on first open and * reusing the same workbench on every later open — `packages/chat/src/ - * routes.ts`'s `POST /workbenches` with `reuseExisting: true` already - * finds-or-creates by `chat/definitionId` (`findExistingAgentChat`), the - * same seam the home-workbench land-hop uses. `tenantId` must be the + * routes.ts`'s `POST /workbenches` always finds-or-creates by + * `chat/definitionId` (`findExistingAgentChat`). `tenantId` must be the * definition's OWNING tenant (see `VisibleAgentDefinition.tenantId`), * never the caller's own tenant when the agent was reached through - * ancestor inheritance — the DM workbench lives where the agent lives. + * ancestor inheritance — the conversation lives where the agent lives. */ -export function openAgentDm( +export function openAgentConversation( tenantId: string, definitionId: string, ): Promise { return createWorkbench(tenantId, { kind: "chat", definitionId, - reuseExisting: true, }); } +export function openAgentDm( + tenantId: string, + definitionId: string, +): Promise { + return openAgentConversation(tenantId, definitionId); +} + export function getAgentInstructions( tenantId: string, definitionId: string, diff --git a/packages/chat-ui/src/default-agent-workbench.test.ts b/packages/chat-ui/src/default-agent-workbench.test.ts index a2ebd8d90..b470fedcd 100644 --- a/packages/chat-ui/src/default-agent-workbench.test.ts +++ b/packages/chat-ui/src/default-agent-workbench.test.ts @@ -122,7 +122,6 @@ describe("createDefaultAgentWorkbench", () => { kind: "chat", definitionId: "def-assistant", name: "Myra", - reuseExisting: true, }); expect(agent.isCachedWorkbenchId("chat-1")).toBe(true); }); diff --git a/packages/chat-ui/src/default-agent-workbench.ts b/packages/chat-ui/src/default-agent-workbench.ts index 72d478f07..aea86ccaa 100644 --- a/packages/chat-ui/src/default-agent-workbench.ts +++ b/packages/chat-ui/src/default-agent-workbench.ts @@ -13,18 +13,11 @@ // participant is a husk that can't answer under either kind, so it is // left alone and the real chat is created. // -// This is the one deliberate find-or-create path in the product (CL-6089): -// the account's home-workbench land-hop (Myra), where landing twice must -// mean the same conversation, never two. Every other agent-chat creation -// — "+ New Workbench" picking an agent as a template, a freshly drafted -// agent's own launch — always mints a new workbench instead. The dedup -// here is `ensure`'s own title match above, not the server's -// `reuseExisting` flag on `POST /workbenches` (see `packages/chat/src/routes.ts` -// `findExistingAgentChat`): by the time `createWorkbench` below is reached, -// `ensure` has already exhausted its own by-title lookup and found no -// match, so a further server-side reuse pass here would only matter for -// a chat renamed away from the agent's title — an edge case this ticket -// leaves as-is rather than threading `reuseExisting` through here too. +// The account's home-workbench land-hop (Myra) still uses this module's +// title match for a fast local reopen. The create at the end does not +// pass a reuse flag: `POST /workbenches` with kind=chat + definitionId +// always find-or-reopens by the definition's asset (CL-6981), so a chat +// renamed away from the agent's title is still the same conversation. import { isAgentAddress } from "@corbits/chat/mentions"; @@ -133,14 +126,13 @@ export function createDefaultAgentWorkbench( message: `No "${config.title}" agent found for this workbench.`, }; } - // The home workbench is the one deliberate reopen: the server's - // definitionId dedup catches it even after a rename, where this - // module's own title lookup above would miss and mint a second. + // The home workbench still titles the mint; the server's + // definitionId/asset dedup catches a rename, where this module's + // own title lookup above would miss. const created = await createWorkbench(tenantId, { kind: "chat", definitionId: definition.id, name: config.title, - reuseExisting: true, }); cachedWorkbenchId = created.id; return { kind: "ready", workbenchId: created.id }; diff --git a/packages/chat-ui/src/index.ts b/packages/chat-ui/src/index.ts index d103a7e04..ad0672765 100644 --- a/packages/chat-ui/src/index.ts +++ b/packages/chat-ui/src/index.ts @@ -151,6 +151,7 @@ export { listInvitableDefinitions, listTenantInvitableDefinitions, listVisibleAgentDefinitions, + openAgentConversation, openAgentDm, inviteAgent, workbenchStreamUrl, diff --git a/packages/chat-ui/test/api.test.ts b/packages/chat-ui/test/api.test.ts index e8dd7408b..caddc8913 100644 --- a/packages/chat-ui/test/api.test.ts +++ b/packages/chat-ui/test/api.test.ts @@ -17,7 +17,7 @@ import { listInvitableDefinitions, listTenantInvitableDefinitions, listVisibleAgentDefinitions, - openAgentDm, + openAgentConversation, listMessages, listPinnedMessages, quickCreateJimmy, @@ -458,8 +458,8 @@ describe("listVisibleAgentDefinitions", () => { }); }); -describe("openAgentDm", () => { - test("creates a chat workbench with reuseExisting so a second open finds the same workbench", async () => { +describe("openAgentConversation", () => { + test("creates a chat workbench so a second open finds the same workbench", async () => { const calls = stubFetch(() => json( { @@ -472,12 +472,11 @@ describe("openAgentDm", () => { 201, ), ); - const workbench = await openAgentDm("tnt_root", "wfd_outreach"); + const workbench = await openAgentConversation("tnt_root", "wfd_outreach"); expect(calls[0]?.path).toBe("/api/tenants/tnt_root/chat/workbenches"); expect(JSON.parse(String(calls[0]?.init?.body))).toEqual({ kind: "chat", definitionId: "wfd_outreach", - reuseExisting: true, }); expect(workbench.id).toBe("c_dm"); }); diff --git a/packages/chat/src/routes.ts b/packages/chat/src/routes.ts index 9e50681d1..6246b3673 100644 --- a/packages/chat/src/routes.ts +++ b/packages/chat/src/routes.ts @@ -322,7 +322,6 @@ const CreateWorkbenchBody = type({ "participants?": "string[]", "definitionId?": "string", "principalId?": "string", - "reuseExisting?": "boolean", /** * The picked template's own promise line * (`WorkbenchTemplateManifest.promise`, see `@corbits/workflow-catalog`), @@ -345,6 +344,7 @@ const CreateWorkbenchBody = type({ * so the caller supplies the one line the card is allowed to show. */ "connectGithubRequiredFor?": "string", + "+": "reject", }); type CreateWorkbenchBodyT = typeof CreateWorkbenchBody.infer; @@ -879,18 +879,10 @@ const MoveWorkbenchBody = type({ }); /** - * Finds an existing chat with the given agent, for the one caller that - * deliberately wants find-or-create semantics: the home-workbench - * land-hop (`ensureMyraWorkbench`, via `default-agent-workbench.ts`), which - * passes `reuseExisting: true` so returning to "Myra" always reopens the - * same conversation rather than minting a fresh one on every visit. - * - * Every other caller — "+ New Workbench" picking an agent as a - * template, or a freshly drafted agent's own launch — always creates - * (CL-6089): the same agent picked twice from the picker is two - * independent workbenches, each with its own workbench tenant and its own - * launched agent instance, not the same conversation reopened. `POST - * /workbenches` only calls this lookup when `reuseExisting` is set. + * Finds an existing chat with the given agent. `POST /workbenches` with + * `kind: "chat"` and a `definitionId` always find-or-reopens (CL-6981): + * identity is the definition's ASSET via `resolveDefinitionAssetId`, and + * the oldest workbench-tenancy `createdAt` wins. No title match. * * Matches forward, by the `chat/definitionId` every agent chat has * carried in its settings since this landed, and falls back to @@ -1060,17 +1052,13 @@ export function createChatRoutes(deps: CreateChatRoutesDeps): Hono { const tenant = c.get("tenant"); const principal = c.get("principal"); - // "+ New Workbench" always creates (CL-6089): picking an agent in - // the picker uses it as a template, minting a fresh workbench - // every time, not reopening a prior conversation. The one - // exception is the deliberate land-hop to the account's home - // workbench (`ensureMyraWorkbench`), which opts in with - // `reuseExisting: true` so landing on "Myra" always finds the - // same conversation instead of forking a new one on every visit. - // Checked before anything is minted, and before the (cheaper, + // Opening an agent always find-or-reopens its one conversation + // (CL-6981): identity is the definition's asset, oldest createdAt + // wins. Checked before anything is minted, and before the (cheaper, // in-memory) principal-self-chat validation below, since a found - // match short-circuits the whole handler. - if (isChatWithDefinition(body) && body.reuseExisting === true) { + // match short-circuits the whole handler — a reopen must not launch + // again. + if (isChatWithDefinition(body)) { const existing = await findExistingAgentChat( deps, tenant.id, diff --git a/packages/chat/test/routes.test.ts b/packages/chat/test/routes.test.ts index 61a481037..b985f34fb 100644 --- a/packages/chat/test/routes.test.ts +++ b/packages/chat/test/routes.test.ts @@ -573,8 +573,8 @@ describe("POST /workbenches", () => { }); }); -describe("POST /workbenches — reuseExisting: true reopens the land-hop's chat, not create (CL-6089)", () => { - test("creating a chat with the same agent twice, reuseExisting: true both times, reuses the first chat instead of forking a duplicate", async () => { +describe("POST /workbenches — opening an agent reopens its one conversation (CL-6981)", () => { + test("POST {kind:chat, definitionId} twice returns the same workbench id (201 then 200)", async () => { const deps = buildDeps({ platform: fakePlatform({ invitable: [{ id: "wfd_echo", name: "Echo" }] }), }); @@ -583,14 +583,12 @@ describe("POST /workbenches — reuseExisting: true reopens the land-hop's chat, const first = await createWorkbench(app, { kind: "chat", definitionId: "wfd_echo", - reuseExisting: true, }); expect(first.response.status).toBe(201); const second = await createWorkbench(app, { kind: "chat", definitionId: "wfd_echo", - reuseExisting: true, }); expect(second.response.status).toBe(200); @@ -601,6 +599,13 @@ describe("POST /workbenches — reuseExisting: true reopens the land-hop's chat, expect(platform.launchInviteCalls).toHaveLength(1); const chats = await deps.store.listWorkbenchSettings(TENANT.id, "chat"); expect(chats).toHaveLength(1); + + const firstTenancy = await deps.tenancy.getWorkbenchTenancy(first.body.id); + const secondTenancy = await deps.tenancy.getWorkbenchTenancy( + second.body.id, + ); + expect(firstTenancy?.tenantId).toBeDefined(); + expect(secondTenancy?.tenantId).toBe(firstTenancy?.tenantId); }); test("reuses the chat when the agent's definition was re-projected under a new id over the same asset", async () => { @@ -619,14 +624,12 @@ describe("POST /workbenches — reuseExisting: true reopens the land-hop's chat, const first = await createWorkbench(app, { kind: "chat", definitionId: "wfd_echo_v1", - reuseExisting: true, }); expect(first.response.status).toBe(201); const second = await createWorkbench(app, { kind: "chat", definitionId: "wfd_echo_v2", - reuseExisting: true, }); expect(second.response.status).toBe(200); @@ -682,7 +685,6 @@ describe("POST /workbenches — reuseExisting: true reopens the land-hop's chat, const { response, body } = await createWorkbench(app, { kind: "chat", definitionId: "wfd_echo", - reuseExisting: true, }); expect(response.status).toBe(200); @@ -746,7 +748,6 @@ describe("POST /workbenches — reuseExisting: true reopens the land-hop's chat, const { response, body } = await createWorkbench(app, { kind: "chat", definitionId: "wfd_echo", - reuseExisting: true, }); expect(response.status).toBe(200); @@ -762,12 +763,10 @@ describe("POST /workbenches — reuseExisting: true reopens the land-hop's chat, const first = await createWorkbench(app, { kind: "chat", definitionId: "wfd_echo", - reuseExisting: true, }); const second = await createWorkbench(app, { kind: "chat", definitionId: "wfd_echo", - reuseExisting: true, }); expect(second.response.status).toBe(200); @@ -780,91 +779,23 @@ describe("POST /workbenches — reuseExisting: true reopens the land-hop's chat, expect(fanOut?.workbenchId).toBe("ins_invited1"); expect(fanOut?.fromWorkbenchId).toBe(first.body.id); }); -}); - -describe("POST /workbenches — agent chat always creates by default (CL-6089)", () => { - test("creating a chat with the same agent twice, reuseExisting omitted both times, mints two independent workbenches", async () => { - const deps = buildDeps({ - platform: fakePlatform({ invitable: [{ id: "wfd_echo", name: "Echo" }] }), - }); - const app = mountAs(createChatRoutes(deps), "prn_alice"); - - const first = await createWorkbench(app, { - kind: "chat", - definitionId: "wfd_echo", - }); - expect(first.response.status).toBe(201); - - const second = await createWorkbench(app, { - kind: "chat", - definitionId: "wfd_echo", - }); - - expect(second.response.status).toBe(201); - expect(second.body.id).not.toBe(first.body.id); - expect(second.body.kind).toBe("chat"); - - const platform = deps.platform as ReturnType; - expect(platform.launchInviteCalls).toHaveLength(2); - const chats = await deps.store.listWorkbenchSettings(TENANT.id, "chat"); - expect(chats).toHaveLength(2); - - // CL-6387: "mints two independent workbenches" must mean two genuinely - // distinct child tenants — not two workbench rows sharing one tenant - // (which would alias participants/grants across both chats). - const firstTenancy = await deps.tenancy.getWorkbenchTenancy(first.body.id); - const secondTenancy = await deps.tenancy.getWorkbenchTenancy( - second.body.id, - ); - expect(firstTenancy?.tenantId).toBeDefined(); - expect(secondTenancy?.tenantId).toBeDefined(); - expect(secondTenancy?.tenantId).not.toBe(firstTenancy?.tenantId); - }); - test("creating a chat with the same agent twice, reuseExisting: false explicitly, still mints two workbenches", async () => { + test("sending reuseExisting is rejected", async () => { const deps = buildDeps({ platform: fakePlatform({ invitable: [{ id: "wfd_echo", name: "Echo" }] }), }); const app = mountAs(createChatRoutes(deps), "prn_alice"); - const first = await createWorkbench(app, { - kind: "chat", - definitionId: "wfd_echo", - reuseExisting: false, - }); - const second = await createWorkbench(app, { + const { response, body } = await createWorkbench(app, { kind: "chat", definitionId: "wfd_echo", reuseExisting: false, }); - expect(first.response.status).toBe(201); - expect(second.response.status).toBe(201); - expect(second.body.id).not.toBe(first.body.id); - }); - - test("a pre-existing chat for the same agent (from an earlier find-or-create call) doesn't stop a later always-create call from minting its own", async () => { - const deps = buildDeps({ - platform: fakePlatform({ invitable: [{ id: "wfd_echo", name: "Echo" }] }), - }); - const app = mountAs(createChatRoutes(deps), "prn_alice"); - - const landHop = await createWorkbench(app, { - kind: "chat", - definitionId: "wfd_echo", - reuseExisting: true, - }); - expect(landHop.response.status).toBe(201); - - const picked = await createWorkbench(app, { - kind: "chat", - definitionId: "wfd_echo", + expect(response.status).toBe(400); + expect(body).toMatchObject({ + error: { code: "bad_request" }, }); - - expect(picked.response.status).toBe(201); - expect(picked.body.id).not.toBe(landHop.body.id); - const chats = await deps.store.listWorkbenchSettings(TENANT.id, "chat"); - expect(chats).toHaveLength(2); }); }); diff --git a/packages/workflow-catalog/src/instantiate.ts b/packages/workflow-catalog/src/instantiate.ts index 68d6d8ca1..5d102acf3 100644 --- a/packages/workflow-catalog/src/instantiate.ts +++ b/packages/workflow-catalog/src/instantiate.ts @@ -76,8 +76,9 @@ export interface WorkbenchTemplateInstantiationPorts { * `@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`. */ + * for Myra: the host invites her separately after minting the room + * (CL-6981 — she must not be the mint `definitionId`, which would + * find-or-reopen her one agent conversation). */ inviteParticipantAgent(id: string): Promise; /** Persists the room's still-needed connections — the workbench * settings `template/pendingConnections` key today; see diff --git a/scripts/e2e/chat.test.ts b/scripts/e2e/chat.test.ts index d7802a9b4..7aa7de0dd 100644 --- a/scripts/e2e/chat.test.ts +++ b/scripts/e2e/chat.test.ts @@ -846,16 +846,13 @@ describe.skipIf(databaseUrl === undefined)("chat e2e", () => { }, 90_000); // Runs after the echo chat above so its own create call — same - // tenant, same agent — proves the deliberate reuse path (CL-6089): - // "+ New Workbench" always creates, so reuse is opt-in via - // `reuseExisting: true` (the land-hop `ensureMyraWorkbench` uses), - // which reopens the existing chat with 200 and the same id back, - // never a fresh 201. + // tenant, same agent — proves find-or-reopen (CL-6981): a second + // POST with kind=chat + definitionId reopens the existing chat with + // 200 and the same id back, never a fresh 201. test("kind filter excludes and includes by kind, and re-creating an existing agent chat reuses it", async () => { const reopened = await createWorkbench({ kind: "chat", definitionId: await echoDefinitionId(), - reuseExisting: true, }); expectStatus("re-create existing agent chat", reopened, 200); expect(