From d687fbe24bc5bc3b212cf3c54d790f4380217fcc Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 27 Aug 2026 10:40:15 -0700 Subject: [PATCH 1/3] Chat: post a room's onboarding card by route, not as a side effect of hosting an agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add POST /workbenches/:id/onboarding, which posts a connect-github block into a hostless channel from a system sender, so a template's walkthrough runs in the empty room it creates. The card carries the definition's title, promise, and ordered step labels. Drop the templatePromise and connectGithubRequiredFor create-body fields and the greet-then-card branch of the hosted mint; the canned greeting is for agent DMs only. Settling a credential a template room is waiting on posts the connected event from the system address and never wakes an agent — a reviewer roster has no host to answer. --- apps/web/src/instant-agent-create.ts | 4 - apps/web/test/new-workbench-picker.test.tsx | 10 - packages/chat-ui/src/api.ts | 53 +++--- packages/chat-ui/src/index.ts | 3 + packages/chat-ui/test/api.test.ts | 43 +++++ packages/chat/src/blocks.ts | 28 +++ packages/chat/src/connect-pending.ts | 17 +- packages/chat/src/index.ts | 2 + packages/chat/src/routes.ts | 97 +++++----- packages/chat/src/workbench-service.ts | 22 --- packages/chat/test/connect-pending.test.ts | 60 +++++- packages/chat/test/routes.test.ts | 185 ++++++++++++------- packages/chat/test/workbench-service.test.ts | 32 ---- packages/evals/src/targets/real-target.ts | 4 - 14 files changed, 338 insertions(+), 222 deletions(-) diff --git a/apps/web/src/instant-agent-create.ts b/apps/web/src/instant-agent-create.ts index 3b56fdc8d..9355225bb 100644 --- a/apps/web/src/instant-agent-create.ts +++ b/apps/web/src/instant-agent-create.ts @@ -189,10 +189,6 @@ export async function createWorkbenchFromTemplate( const workbench = await createWorkbench(tenantId, { kind: "workbench", name: manifest?.title ?? NEW_WORKBENCH_TITLE, - ...(manifest !== undefined ? { templatePromise: manifest.promise } : {}), - ...(requiresGithub && !githubAlreadyConnected && manifest !== undefined - ? { connectGithubRequiredFor: manifest.title } - : {}), }); if (githubAlreadyConnected && pickGithubRepos !== undefined) { diff --git a/apps/web/test/new-workbench-picker.test.tsx b/apps/web/test/new-workbench-picker.test.tsx index 9370b901a..c1ba6ed5e 100644 --- a/apps/web/test/new-workbench-picker.test.tsx +++ b/apps/web/test/new-workbench-picker.test.tsx @@ -563,8 +563,6 @@ describe("NewWorkbenchPickerRoute", () => { ); expect(JSON.parse(String(createWorkbenchCall?.init?.body))).toMatchObject({ kind: "workbench", - templatePromise: - "Three reviewers read every pull request and post what they'd change.", }); expect( JSON.parse(String(createWorkbenchCall?.init?.body)).definitionId, @@ -750,14 +748,6 @@ describe("NewWorkbenchPickerRoute", () => { expect(navigated).toEqual(["/w/chan_new"]); - const createWorkbenchCall = calls.find( - (call) => - call.path.endsWith("/chat/workbenches") && call.init?.method === "POST", - ); - expect( - JSON.parse(String(createWorkbenchCall?.init?.body)), - ).not.toHaveProperty("connectGithubRequiredFor"); - const startReviewingCall = calls.find((call) => call.path.endsWith("/workbenches/chan_new/github/start-reviewing"), ); diff --git a/packages/chat-ui/src/api.ts b/packages/chat-ui/src/api.ts index 52a276d08..8e849c3d3 100644 --- a/packages/chat-ui/src/api.ts +++ b/packages/chat-ui/src/api.ts @@ -13,6 +13,7 @@ import type { ArkErrors } from "arktype"; import { Part } from "@corbits/chat/parts"; import { parseParticipants } from "@corbits/chat/participants"; import type { ParticipantRecord } from "@corbits/chat/participants"; +import type { WorkbenchOnboardingStep } from "@corbits/chat/blocks"; import { UnauthenticatedError } from "@corbits/api-query"; import { jimmyAgentRequest } from "@corbits/workflow-catalog"; import { CHAT_STRINGS } from "./strings"; @@ -27,6 +28,10 @@ export { Part, } from "@corbits/chat/parts"; export type { ParticipantRecord } from "@corbits/chat/participants"; +export type { + OnboardingStepLabel, + WorkbenchOnboardingStep, +} from "@corbits/chat/blocks"; export { REACTION_EMOJI } from "@corbits/chat/reaction-emoji"; export type { ReactionEmoji } from "@corbits/chat/reaction-emoji"; @@ -335,41 +340,19 @@ export function listAllWorkbenches( // // `kind: "chat"` + `definitionId` always find-or-reopens the one DM // for that agent (CL-6981). `reuseExisting` is still accepted on the -// wire and ignored. `kind: "workbench"` mints an empty channel; named -// templates may send `templatePromise` / `connectGithubRequiredFor` so -// the opener and GitHub card can follow the roster invite. +// wire and ignored. `kind: "workbench"` mints an empty channel; a room's +// onboarding walkthrough is posted separately through +// `postWorkbenchOnboardingStep`, never as a side effect of create. export type CreateWorkbenchInput = | { readonly kind: "workbench"; readonly name: string; - /** Named-template opener line. The server currently posts the - * canned greeting only on `kind: "chat"` + `definitionId`; this - * field is still sent so a follow-up can honor it on channel - * mint / first invite without dropping the promise from the - * create body. Omitted for a blank channel. */ - readonly templatePromise?: string; - /** Template display name when GitHub must be connected before - * the roster can run. Omitted when the template does not - * require GitHub. */ - readonly connectGithubRequiredFor?: 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 - * actual job (`packages/chat/src/routes.ts`'s `POST /workbenches`). - * Omitted for an untemplated chat. */ - readonly templatePromise?: string; - /** The template's own display name, present exactly when the - * template needs a GitHub connection before it can run — posts one - * `connect-github` block right after the canned greeting - * (`packages/chat/src/routes.ts`'s `POST /workbenches`). Omitted - * for a template with no such requirement. */ - readonly connectGithubRequiredFor?: string; } | { readonly kind: "chat"; @@ -711,6 +694,26 @@ export function inviteAgent( ); } +const PostedOnboardingStep = type({ id: "string" }); + +/** + * Posts one onboarding step into a room + * (`POST /workbenches/:id/onboarding`): the walkthrough card lands as a + * system row, with no agent launched or woken, so an empty channel can + * run its onboarding with nobody in the room yet. + */ +export function postWorkbenchOnboardingStep( + tenantId: string, + workbenchId: string, + step: WorkbenchOnboardingStep, +): Promise<{ readonly id: string }> { + return request( + `/api/tenants/${tenantId}/chat/workbenches/${workbenchId}/onboarding`, + PostedOnboardingStep, + { method: "POST", body: JSON.stringify(step) }, + ); +} + // Jimmy's own request shape, the same `@corbits/workflow-catalog` object a // workbench template's participant create used to resolve — CL-6499 removed // Jimmy's template (he is not a "kind of workbench"), so this dialog's own diff --git a/packages/chat-ui/src/index.ts b/packages/chat-ui/src/index.ts index 512a856c0..1c8b4c5a2 100644 --- a/packages/chat-ui/src/index.ts +++ b/packages/chat-ui/src/index.ts @@ -161,6 +161,7 @@ export { getWorkbenchSettings, patchWorkbenchSettings, getConnectGithubState, + postWorkbenchOnboardingStep, startReviewingGithubRepos, type ConnectGithubStateResponse, getBenchChatSettings, @@ -179,6 +180,8 @@ export { export type { Workbench, CreateWorkbenchInput, + OnboardingStepLabel, + WorkbenchOnboardingStep, ParticipantRecord, MessageItem, MessagesResponse, diff --git a/packages/chat-ui/test/api.test.ts b/packages/chat-ui/test/api.test.ts index e8dd7408b..2094503cf 100644 --- a/packages/chat-ui/test/api.test.ts +++ b/packages/chat-ui/test/api.test.ts @@ -28,6 +28,7 @@ import { getBenchChatSettings, patchBenchChatSettings, pinMessage, + postWorkbenchOnboardingStep, toggleReaction, unpinMessage, } from "../src/api"; @@ -506,6 +507,48 @@ describe("inviteAgent", () => { }); }); +describe("postWorkbenchOnboardingStep", () => { + test("posts the step to the workbench's onboarding route and parses the posted id", async () => { + const calls = stubFetch(() => json({ id: "msg_1" }, 201)); + const step = { + kind: "connect-github" as const, + requiredForTemplate: "Code review", + promise: "Three reviewers read every pull request.", + steps: [ + { title: "Connect GitHub", why: "So reviewers can read your code." }, + ], + }; + + const posted = await postWorkbenchOnboardingStep( + "tenant_1", + "chan_1", + step, + ); + + expect(calls[0]?.path).toBe( + "/api/tenants/tenant_1/chat/workbenches/chan_1/onboarding", + ); + expect(calls[0]?.init?.method).toBe("POST"); + expect(JSON.parse(String(calls[0]?.init?.body))).toEqual(step); + expect(posted).toEqual({ id: "msg_1" }); + }); + + test("a rejected step surfaces as a ChatApiError", async () => { + stubFetch(() => + json({ error: { code: "bad_request", message: "nope" } }, 400), + ); + + await expect( + postWorkbenchOnboardingStep("tenant_1", "chan_1", { + kind: "connect-github", + requiredForTemplate: "Code review", + promise: "Three reviewers read every pull request.", + steps: [], + }), + ).rejects.toBeInstanceOf(ChatApiError); + }); +}); + describe("quickCreateJimmy", () => { test("posts Jimmy's own request shape to the agent-definitions create route", async () => { const calls = stubFetch(() => json({ id: "wfd_jimmy" }, 201)); diff --git a/packages/chat/src/blocks.ts b/packages/chat/src/blocks.ts index feff0b306..7e109548a 100644 --- a/packages/chat/src/blocks.ts +++ b/packages/chat/src/blocks.ts @@ -128,15 +128,43 @@ export type QuestionBlockData = typeof QuestionBlockData.infer; // the connected verdict itself — comes from a host-supplied actions // port at render time, resolved against the room's real connection and // settings, never from this data. +// One labelled step of a room's onboarding walkthrough: what the person +// does, and why it matters to them. The definition that owns the +// walkthrough owns the copy — the card renders its step rail from these +// labels rather than holding step text of its own. +export const OnboardingStepLabel = type({ + title: "string > 0", + why: "string > 0", +}).onDeepUndeclaredKey("delete"); +export type OnboardingStepLabel = typeof OnboardingStepLabel.infer; + +// The body `POST /workbenches/:id/onboarding` parses: one declared +// onboarding step a host may post into a room, never an arbitrary block. +// A `.or(...)` here is the extension point when a second kind of step +// earns one. +export const WorkbenchOnboardingStep = type({ + kind: "'connect-github'", + requiredForTemplate: "string > 0", + promise: "string > 0", + steps: OnboardingStepLabel.array(), +}).onDeepUndeclaredKey("delete"); +export type WorkbenchOnboardingStep = typeof WorkbenchOnboardingStep.infer; + +// `promise` and `steps` are optional so cards persisted before the +// onboarding route existed still parse; the route always writes both. const ConnectGithubDisconnectedData = type({ requiredForTemplate: "string > 0", state: "'disconnected'", + "promise?": "string > 0", + "steps?": OnboardingStepLabel.array(), }).onDeepUndeclaredKey("delete"); const ConnectGithubConnectedData = type({ requiredForTemplate: "string > 0", state: "'connected'", orgName: "string", + "promise?": "string > 0", + "steps?": OnboardingStepLabel.array(), }).onDeepUndeclaredKey("delete"); export const ConnectGithubBlockData = ConnectGithubDisconnectedData.or( diff --git a/packages/chat/src/connect-pending.ts b/packages/chat/src/connect-pending.ts index 96d24f597..2ac18d66b 100644 --- a/packages/chat/src/connect-pending.ts +++ b/packages/chat/src/connect-pending.ts @@ -15,7 +15,10 @@ // Plugins page, another tab) never reached it. Rather than stand up a // second settle path for that one key, this module settles both: a // connector becoming connected is one event, and every room's settling -// belongs to one mechanism, not two parallel key conventions. +// belongs to one mechanism, not two parallel key conventions. A +// template-key-only match settles and notices without waking anyone — +// only a room whose own `connections/pending` named the connector had an +// agent waiting on it. import { type } from "arktype"; import { localPartOf } from "./agent-address"; @@ -187,9 +190,17 @@ export async function settleConnectedService( data: { updatedBy: input.principalId, settings: updated.settings }, }); - const agentAddress = hostAgentAddress(updated.settings, input.principalId); + // A room matched only through the template key never wakes an agent: + // its walkthrough was posted by the product, not asked for by an + // agent mid-turn, and the room's first agent participant may be a + // reviewer whose prompt only speaks JSON. A room whose own + // `connections/pending` names the connector did have an agent ask + // for it, so that agent still gets woken. + const agentAddress = matchedPending + ? hostAgentAddress(updated.settings, input.principalId) + : undefined; // CL-6741: event-only system row — never a signed-in user text bubble. - // Sender is the host agent when one exists; otherwise a synthetic + // Sender is the woken agent when there is one; otherwise a synthetic // system address so the row never attributes to the connecting person. await postRoomMessage(deps, { tenantId: input.tenantId, diff --git a/packages/chat/src/index.ts b/packages/chat/src/index.ts index f872d4ae6..fd9692b23 100644 --- a/packages/chat/src/index.ts +++ b/packages/chat/src/index.ts @@ -19,6 +19,8 @@ export { StreamBlockData, QuestionBlockData, ConnectServiceBlockData, + OnboardingStepLabel, + WorkbenchOnboardingStep, parseBlock, } from "./blocks"; export type { Block, BlockParseResult } from "./blocks"; diff --git a/packages/chat/src/routes.ts b/packages/chat/src/routes.ts index d95a3db10..196da3ab5 100644 --- a/packages/chat/src/routes.ts +++ b/packages/chat/src/routes.ts @@ -64,6 +64,7 @@ import { } from "./workbench-settings"; import { isRecentlyActive } from "./workbench-activity"; import { postRoomMessage, type RoomMessageStore } from "./room-messages"; +import { WorkbenchOnboardingStep } from "./blocks"; import type { ConnectGithubBlockData } from "./blocks"; import { findResidentAgentForDefinition, @@ -329,28 +330,6 @@ const CreateWorkbenchBody = type({ * `openAgentDm`) are not 400'd. */ "reuseExisting?": "boolean", - /** - * The picked template's own promise line - * (`WorkbenchTemplateManifest.promise`, see `@corbits/workflow-catalog`), - * when this chat was minted from the `/new` picker's template - * instantiation flow (`apps/web/src/instant-agent-create.ts`). Passed - * through as an opaque string — this package has no notion of a - * template — to replace the random canned opener with one naming the - * room's actual job. Omitted mints exactly like an untemplated chat. - */ - "templatePromise?": "string", - /** - * The template's own display name, present exactly when this chat's - * template needs a GitHub connection before it can run - * (`WorkbenchTemplateManifest.requiredConnections` naming `"github"` — - * see `apps/web/src/instant-agent-create.ts`). Posts one - * `connect-github` block (`./blocks.ts`) right after the canned - * greeting, in `state: "disconnected"` — this package owns the block - * vocabulary generically (the same way `chat-orchestrator.ts` posts an - * `approve` block), but has no notion of *why* a template needs GitHub, - * so the caller supplies the one line the card is allowed to show. - */ - "connectGithubRequiredFor?": "string", }); type CreateWorkbenchBodyT = typeof CreateWorkbenchBody.infer; @@ -1284,8 +1263,6 @@ export function createChatRoutes(deps: CreateChatRoutesDeps): Hono { const agentAddress = joined.address; const joinEventDelivered = joined.joinEventDelivered; const agentDisplayName = joined.displayName; - const templatePromise = body.templatePromise; - const connectGithubRequiredFor = body.connectGithubRequiredFor; runPostMintDelivery(async () => { const senderName = deps.resolvePrincipalName !== undefined @@ -1304,27 +1281,8 @@ export function createChatRoutes(deps: CreateChatRoutesDeps): Hono { agentAddress, agentName: agentDisplayName, ...(senderName !== undefined ? { senderName } : {}), - ...(templatePromise !== undefined ? { templatePromise } : {}), }, ); - if (connectGithubRequiredFor !== undefined) { - const data: ConnectGithubBlockData = { - requiredForTemplate: connectGithubRequiredFor, - state: "disconnected", - }; - await postRoomMessage( - { roomMessages: deps.roomMessages, publish }, - { - tenantId: tenant.id, - workbenchId, - sender: { name: null, address: agentAddress }, - runId: localPartOf(agentAddress), - parts: [ - { kind: "block", block: { type: "connect-github", data } }, - ], - }, - ); - } await deps.platform .ensureAwake(agentAddress) .catch((err: unknown) => { @@ -2875,6 +2833,59 @@ export function createChatRoutes(deps: CreateChatRoutesDeps): Hono { }, ); + // A room's onboarding walkthrough, posted explicitly by whoever knows + // what the room is for — never as a side effect of hosting an agent. + // The step lands as a system row (no run, no launch, no wake), so an + // empty channel can run its walkthrough with no agent in the room at + // all. Only the declared step shapes are accepted: this is not a + // general "post any block" hole in the route surface. + app.post( + "/workbenches/:id/onboarding", + deps.requireGrant(idResource("workflow-run", "id"), "create"), + async (c) => { + const step = WorkbenchOnboardingStep( + await c.req.json().catch(() => undefined), + ); + if (step instanceof type.errors) { + return c.json( + ErrorEnvelope( + "bad_request", + `invalid onboarding step: ${step.summary}`, + ), + 400, + ); + } + + const tenant = c.get("tenant"); + const workbenchId = c.req.param("id"); + const existing = await deps.store.getWorkbenchSettings( + tenant.id, + workbenchId, + ); + if (existing === undefined) { + return c.json(ErrorEnvelope("not_found", "workbench not found"), 404); + } + + const data: ConnectGithubBlockData = { + requiredForTemplate: step.requiredForTemplate, + promise: step.promise, + steps: step.steps, + state: "disconnected", + }; + const posted = await postRoomMessage( + { roomMessages: deps.roomMessages, publish }, + { + tenantId: tenant.id, + workbenchId, + sender: { name: null, address: `system@${workbenchId}` }, + parts: [{ kind: "block", block: { type: "connect-github", data } }], + }, + ); + + return c.json({ id: posted.id }, 201); + }, + ); + // The removal counterpart to `POST .../invite` (and to the inline // join a chat's own creation runs): drops a participant record and, // for an invited agent, releases its launched instance — see diff --git a/packages/chat/src/workbench-service.ts b/packages/chat/src/workbench-service.ts index 859d311b1..ee38afc31 100644 --- a/packages/chat/src/workbench-service.ts +++ b/packages/chat/src/workbench-service.ts @@ -393,14 +393,6 @@ export type CannedGreetingInput = { readonly agentName: string; /** The opener's display name, when the host can resolve one. */ readonly senderName?: string; - /** - * The picked template's promise line (`WorkbenchTemplateManifest.promise` - * — see `@corbits/workflow-catalog`), when this chat was minted from - * one. Present, the opener names what the room is actually for - * instead of a generic hello; absent, `GREETING_VARIATIONS` picks the - * usual random opener. - */ - readonly templatePromise?: string; }; export type PostCannedGreetingInput = CannedGreetingInput & { @@ -458,17 +450,6 @@ function greetingVariationIndex(workbenchId: string): number { return sum; } -/** The template-flavored opener: names the room's actual job (the - * manifest's own `promise` line) instead of a generic hello, and asks - * for the one thing every template needs before it can start — - * something connected. */ -function templateGreeting(who: string, agent: string, promise: string): string { - return ( - `Hi${who}, I'm ${agent}. ${promise} Connect what I need and tell me ` + - "what to watch, and I'll take it from there." - ); -} - export function cannedGreeting(input: CannedGreetingInput): string { // The agent states its own name here, verbatim — the exact spot // CL-6471's "I'm run_737a058d…" leaked from. Guarded at the source @@ -479,9 +460,6 @@ export function cannedGreeting(input: CannedGreetingInput): string { input.senderName !== undefined && input.senderName !== "" ? ` ${input.senderName}` : ""; - if (input.templatePromise !== undefined) { - return templateGreeting(who, input.agentName, input.templatePromise); - } const variation = GREETING_VARIATIONS[greetingVariationIndex(input.workbenchId)]; if (variation === undefined) throw new Error("no greeting variations"); diff --git a/packages/chat/test/connect-pending.test.ts b/packages/chat/test/connect-pending.test.ts index 7012922a1..e05ab140d 100644 --- a/packages/chat/test/connect-pending.test.ts +++ b/packages/chat/test/connect-pending.test.ts @@ -2,8 +2,10 @@ // connection completing in the browser settles every room that was waiting // on it — the pending entry clears, `chat.settings` fires so the card // flips, an event-only system notice lands on the timeline (CL-6741), and -// the host agent is woken via `dispatchTurn` / `sendMail` without a new -// timeline row authored as the signed-in user. +// the agent that asked for the connector is woken via `dispatchTurn` / +// `sendMail` without a new timeline row authored as the signed-in user. A +// room matched only through the template's own pending key settles without +// waking anyone. import { expect, test } from "bun:test"; import { createInMemoryAgentTurnStore } from "../src/agent-turns"; @@ -262,7 +264,7 @@ test("matches a pending mcp-prefixed entry when the preset connects under its ba expectEventOnlySettleNotice(listed.items, "Notion"); }); -test("settles a room whose GitHub card is pending under the code-review template's own key — a credential created out of band (not through that card's own submit) still reaches it", async () => { +test("settles a room whose GitHub card is pending under the code-review template's own key — a credential created out of band (not through that card's own submit) still reaches it, and no agent is woken", async () => { const { store, roomMessages, published, platform, agentTurns, deps } = buildDeps(); await seedTemplateWorkbench(store, "chan_template", ["github"]); @@ -294,18 +296,58 @@ test("settles a room whose GitHub card is pending under the code-review template }); expect(listed.items).toHaveLength(1); expectEventOnlySettleNotice(listed.items, "GitHub"); + // The room's walkthrough was posted by the product, not asked for by an + // agent mid-turn: the notice comes from the system address, and the + // room's first agent participant is never dispatched a turn. + expect(listed.items[0]?.sender.address).toBe("system@chan_template"); - expect(platform.sentMail).toHaveLength(1); - expect(platform.sentMail[0]?.workbenchId).toBe("ins_myra"); - expect(platform.sentMail[0]?.fromWorkbenchId).toBe("chan_template"); - expect(platform.sentMail[0]?.content.content).toContain("GitHub"); - + expect(platform.sentMail).toHaveLength(0); const turns = await agentTurns.listTurns({ tenantId: TENANT.id, workbenchId: "chan_template", }); + expect(turns).toHaveLength(0); +}); + +test("a room pending under both keys still wakes its host agent", async () => { + const { store, roomMessages, platform, agentTurns, deps } = buildDeps(); + await store.createWorkbenchSettings({ + tenantId: TENANT.id, + workbenchId: "chan_both", + settings: { + "chat/kind": "workbench", + "chat/participants": [ + { address: HUMAN_ADDRESS, handle: "owner" }, + { address: AGENT_ADDRESS, handle: "myra" }, + ], + "connections/pending": ["github"], + "template/pendingConnections": ["github"], + }, + updatedBy: "prn_owner", + }); + + await settleConnectedService(deps, { + tenantId: TENANT.id, + principalId: "prn_owner", + connectorId: "github", + displayName: "GitHub", + }); + + const settled = await store.getWorkbenchSettings(TENANT.id, "chan_both"); + expect(settled?.settings["connections/pending"]).toEqual([]); + expect(settled?.settings["template/pendingConnections"]).toEqual([]); + + const listed = await roomMessages.listMessages({ + tenantId: TENANT.id, + workbenchId: "chan_both", + }); + expect(listed.items[0]?.sender.address).toBe(AGENT_ADDRESS); + expect(platform.sentMail).toHaveLength(1); + const turns = await agentTurns.listTurns({ + tenantId: TENANT.id, + workbenchId: "chan_both", + }); expect(turns[0]?.agentAddress).toBe(AGENT_ADDRESS); - expect(turns[0]?.requestMessageIds).toEqual([]); }); test("System / settle notices are not presented as the human's messages", async () => { diff --git a/packages/chat/test/routes.test.ts b/packages/chat/test/routes.test.ts index 55358d0dc..6ed923f0c 100644 --- a/packages/chat/test/routes.test.ts +++ b/packages/chat/test/routes.test.ts @@ -249,35 +249,6 @@ describe("POST /workbenches", () => { expect(timelineTexts(timeline)[0]).toMatch(/\?$/); }); - test("creating a chat with a templatePromise greets with it, not a random opener", async () => { - const deliveries: (() => Promise)[] = []; - const deps = buildDeps({ - platform: fakePlatform({ invitable: [{ id: "wfd_echo", name: "Echo" }] }), - runPostMintDelivery: (work) => { - deliveries.push(work); - }, - }); - const app = mountAs(createChatRoutes(deps), "prn_alice"); - - const { body } = await createWorkbench(app, { - kind: "chat", - definitionId: "wfd_echo", - templatePromise: - "Three reviewers read every pull request and post what they'd change.", - }); - await deliveries[0]?.(); - - // The greeting is a row on the chat's own timeline, not mail: the - // only thing the platform is ever asked for on a mint is the - // agent's launch. - const platform = deps.platform as ReturnType; - expect(platform.sentMail).toHaveLength(0); - const timeline = await timelineOf(deps, body.id); - expect(timelineTexts(timeline)[0]).toContain( - "Three reviewers read every pull request and post what they'd change.", - ); - }); - // CL-6471: the owner's live repro — instantiating the code-review // template on a fresh stack, the setup agent's own definition missed // the pre-fetched `invitable` snapshot (a just-seeded/just-redeployed @@ -304,54 +275,15 @@ describe("POST /workbenches", () => { const { body } = await createWorkbench(app, { kind: "chat", definitionId: "wfd_echo", - templatePromise: - "Three reviewers read every pull request and post what they'd change.", }); await deliveries[0]?.(); const timeline = await timelineOf(deps, body.id); - expect(timelineTexts(timeline)[0]).toContain("I'm Myra"); + expect(timelineTexts(timeline)[0]).toContain("Myra"); expect(timelineTexts(timeline)[0]).not.toContain("ins_invited1"); }); - test("creating a chat with connectGithubRequiredFor posts a connect-github block after the greeting", async () => { - const deliveries: (() => Promise)[] = []; - const deps = buildDeps({ - platform: fakePlatform({ invitable: [{ id: "wfd_echo", name: "Echo" }] }), - runPostMintDelivery: (work) => { - deliveries.push(work); - }, - }); - const app = mountAs(createChatRoutes(deps), "prn_alice"); - - const { body } = await createWorkbench(app, { - kind: "chat", - definitionId: "wfd_echo", - templatePromise: "Three reviewers read every pull request.", - connectGithubRequiredFor: "Code review", - }); - await deliveries[0]?.(); - - const timeline = await timelineOf(deps, body.id); - const blockMessage = timeline.find((message) => - message.parts.some( - (part) => part.kind === "block" && part.block.type === "connect-github", - ), - ); - expect(blockMessage).toBeDefined(); - const blockPart = blockMessage?.parts.find( - (part) => part.kind === "block" && part.block.type === "connect-github", - ); - expect(blockPart).toMatchObject({ - kind: "block", - block: { - type: "connect-github", - data: { requiredForTemplate: "Code review", state: "disconnected" }, - }, - }); - }); - - test("creating a chat with no templatePromise mints exactly as it always has", async () => { + test("creating an untemplated chat mints exactly as it always has", async () => { const deliveries: (() => Promise)[] = []; const deps = buildDeps({ platform: fakePlatform({ invitable: [{ id: "wfd_echo", name: "Echo" }] }), @@ -1066,6 +998,119 @@ describe("POST /workbenches/:id/invite", () => { }); }); +describe("POST /workbenches/:id/onboarding", () => { + const STEP = { + kind: "connect-github", + requiredForTemplate: "Code review", + promise: "Three reviewers read every pull request.", + steps: [ + { title: "Connect GitHub", why: "So reviewers can read your code." }, + { title: "Pick repositories", why: "So reviews land where you work." }, + ], + }; + + async function postOnboarding( + app: ReturnType, + workbenchId: string, + body: unknown, + ) { + return app.request(`/workbenches/${workbenchId}/onboarding`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); + } + + test("posts the walkthrough card into an empty channel from a system sender, launching nobody", async () => { + const deps = buildDeps(); + const app = mountAs(createChatRoutes(deps), "prn_alice"); + const { body: workbench } = await createWorkbench(app, { + kind: "workbench", + name: "Code review", + }); + + const response = await postOnboarding(app, workbench.id, STEP); + + expect(response.status).toBe(201); + const posted = (await response.json()) as { id: string }; + expect(typeof posted.id).toBe("string"); + + const timeline = await timelineOf(deps, workbench.id); + expect(timeline).toHaveLength(1); + const message = timeline[0]; + expect(message?.id).toBe(posted.id); + expect(message?.sender.address).toBe(`system@${workbench.id}`); + expect(message?.runId).toBeNull(); + expect(message?.parts).toEqual([ + { + kind: "block", + block: { + type: "connect-github", + data: { + requiredForTemplate: "Code review", + promise: "Three reviewers read every pull request.", + steps: STEP.steps, + state: "disconnected", + }, + }, + }, + ]); + + const platform = deps.platform as ReturnType; + expect(platform.launchInviteCalls).toEqual([]); + expect(platform.ensureAwakeCalls).toEqual([]); + expect(platform.sentMail).toHaveLength(0); + }); + + test("a malformed step is rejected with the structured error envelope", async () => { + const deps = buildDeps(); + const app = mountAs(createChatRoutes(deps), "prn_alice"); + const { body: workbench } = await createWorkbench(app, { + kind: "workbench", + name: "Code review", + }); + + const response = await postOnboarding(app, workbench.id, { + kind: "connect-github", + requiredForTemplate: "Code review", + promise: "", + steps: [], + }); + + expect(response.status).toBe(400); + const errorBody = (await response.json()) as { error: { code: string } }; + expect(errorBody.error.code).toBe("bad_request"); + expect(await timelineOf(deps, workbench.id)).toHaveLength(0); + }); + + test("an undeclared step kind is rejected — the route is not a post-any-block hole", async () => { + const app = mountAs(createChatRoutes(buildDeps()), "prn_alice"); + const { body: workbench } = await createWorkbench(app, { + kind: "workbench", + name: "Code review", + }); + + const response = await postOnboarding(app, workbench.id, { + kind: "approve", + requiredForTemplate: "Code review", + promise: "Anything at all.", + steps: [], + }); + + expect(response.status).toBe(400); + }); + + test("an unknown workbench is a 404", async () => { + const app = mountAs(createChatRoutes(buildDeps()), "prn_alice"); + + const response = await postOnboarding(app, "chan_missing", STEP); + + expect(response.status).toBe(404); + const errorBody = (await response.json()) as { error: { code: string } }; + expect(errorBody.error.code).toBe("not_found"); + }); +}); + describe("DELETE /workbenches/:id/participants/:address", () => { test("removes a human participant and releases nothing (no instance to release)", async () => { const deps = buildDeps(); diff --git a/packages/chat/test/workbench-service.test.ts b/packages/chat/test/workbench-service.test.ts index 79dadfa39..70fa21472 100644 --- a/packages/chat/test/workbench-service.test.ts +++ b/packages/chat/test/workbench-service.test.ts @@ -127,38 +127,6 @@ describe("postCannedGreeting (CL-6126)", () => { }, ); - test("a template promise replaces the random opener with one naming the room's job", () => { - const greeting = cannedGreeting({ - workbenchId: "chan_1", - agentName: "Myra", - senderName: "Ada", - templatePromise: - "Three reviewers read every pull request and post what they'd change.", - }); - expect(greeting).toContain("Ada"); - expect(greeting).toContain("Myra"); - expect(greeting).toContain( - "Three reviewers read every pull request and post what they'd change.", - ); - }); - - test("a template promise is deterministic across workbenches, unlike the random variations", () => { - const promise = "Three reviewers read every pull request."; - expect( - cannedGreeting({ - workbenchId: "chan_1", - agentName: "Myra", - templatePromise: promise, - }), - ).toBe( - cannedGreeting({ - workbenchId: "chan_2", - agentName: "Myra", - templatePromise: promise, - }), - ); - }); - test("a post failure is swallowed, never thrown", async () => { const roomMessages = createInMemoryRoomMessageStore(); roomMessages.insertMessage = async () => { diff --git a/packages/evals/src/targets/real-target.ts b/packages/evals/src/targets/real-target.ts index 8fe889227..de64dcbfc 100644 --- a/packages/evals/src/targets/real-target.ts +++ b/packages/evals/src/targets/real-target.ts @@ -701,11 +701,7 @@ export async function bootMyraTarget( kind: "chat", definitionId: assistantDefinitionId, name: "New Workbench", - templatePromise: manifest.promise, }; - if (manifest.requiredConnections.includes("github")) { - createBody["connectGithubRequiredFor"] = manifest.title; - } const createRes = await api( hub.baseUrl, "POST", From ae38ae47308b164dea7ea9315fdf6561e46d2c05 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 27 Aug 2026 10:57:33 -0700 Subject: [PATCH 2/3] Workbench Definition owns template create MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One arktype WorkbenchDefinition now describes what a template creates: default agents, routines, tools, plugins, and ordered onboarding steps. The shipped manifests are that type; validation fails loud when a required plugin has no connect step, a step names something the definition does not carry, or the walkthrough is out of order. Instantiation gains a typed beginOnboarding port and no longer emits "todo" notes about webhook triggers. The web create flow mints an empty channel with no host, instantiates the definition, then posts its walkthrough into the room — Code review is three reviewers and a GitHub walkthrough, with no Myra. The /new repo dialog for an already-connected GitHub is gone: the in-room card reads connected state live and flips straight to the repo pick, so there is one walkthrough, not two. --- apps/web/src/instant-agent-create.test.ts | 144 ++++++- apps/web/src/instant-agent-create.ts | 173 ++++---- .../src/pages/github-repo-select-dialog.tsx | 72 ---- apps/web/src/pages/new-workbench-picker.tsx | 37 -- apps/web/src/workbench-templates-api.ts | 14 +- apps/web/test/new-workbench-picker.test.tsx | 115 ++---- packages/evals/src/targets/real-target.ts | 44 +- packages/workflow-catalog/README.md | 16 +- packages/workflow-catalog/src/index.ts | 11 +- packages/workflow-catalog/src/instantiate.ts | 158 ++++--- packages/workflow-catalog/src/templates.ts | 391 ++++++++++++------ .../workflow-catalog/test/instantiate.test.ts | 81 +++- .../workflow-catalog/test/templates.test.ts | 162 ++++++-- 13 files changed, 824 insertions(+), 594 deletions(-) delete mode 100644 apps/web/src/pages/github-repo-select-dialog.tsx diff --git a/apps/web/src/instant-agent-create.test.ts b/apps/web/src/instant-agent-create.test.ts index c13f3ca22..7e6221a9c 100644 --- a/apps/web/src/instant-agent-create.test.ts +++ b/apps/web/src/instant-agent-create.test.ts @@ -2,7 +2,7 @@ import { afterEach, describe, expect, test } from "bun:test"; import { QueryClient } from "@tanstack/react-query"; import { CODE_REVIEW_TEMPLATE, - serializeWorkbenchTemplateManifest, + serializeWorkbenchDefinition, } from "@corbits/workflow-catalog"; import { @@ -16,7 +16,7 @@ function newQueryClient(): QueryClient { }); } -describe("createWorkbenchFromTemplate (CL-6387)", () => { +describe("createWorkbenchFromTemplate", () => { const realFetch = globalThis.fetch; afterEach(() => { @@ -146,11 +146,7 @@ describe("createWorkbenchFromTemplate (CL-6387)", () => { expect(calls.some((call) => call.path.includes("/invite"))).toBe(false); }); - // 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 () => { + test("picking the code-review definition names the bench after it and invites exactly its three reviewers", async () => { const navigated: string[] = []; let nextReviewerId = 0; const calls = stubFetch((path) => { @@ -160,7 +156,7 @@ describe("createWorkbenchFromTemplate (CL-6387)", () => { if (path.endsWith("/library/templates/code-review")) { return json({ id: "code-review", - content: serializeWorkbenchTemplateManifest(CODE_REVIEW_TEMPLATE), + content: serializeWorkbenchDefinition(CODE_REVIEW_TEMPLATE), }); } if (path.endsWith("/chat/workbenches")) { @@ -182,6 +178,9 @@ describe("createWorkbenchFromTemplate (CL-6387)", () => { id: `def-reviewer-${nextReviewerId}`, }); } + if (path.endsWith("/chat/workbenches/chan-1/onboarding")) { + return json({ id: "msg-onboarding" }, 201); + } if (path.endsWith("/chat/workbenches/chan-1/invite")) { return json({ address: "agent:invited", definitionId: "def-reviewer" }); } @@ -222,10 +221,7 @@ describe("createWorkbenchFromTemplate (CL-6387)", () => { 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); + expect(createAgentCalls).toHaveLength(CODE_REVIEW_TEMPLATE.agents.length); const inviteCalls = calls.filter((call) => call.path.endsWith("/chat/workbenches/chan-1/invite"), @@ -236,7 +232,37 @@ describe("createWorkbenchFromTemplate (CL-6387)", () => { const createdIds = createAgentCalls.map( (_, index) => `def-reviewer-${index + 1}`, ); - expect(invitedIds.sort()).toEqual(["def-assistant", ...createdIds].sort()); + expect(invitedIds.sort()).toEqual([...createdIds].sort()); + expect(invitedIds).not.toContain("def-assistant"); + + const settingsBody = JSON.parse( + String( + calls.find((call) => + call.path.endsWith("/chat/workbenches/chan-1/settings"), + )?.init?.body, + ), + ); + expect(settingsBody).toEqual({ + "template/id": "code-review", + "template/pendingConnections": ["github"], + }); + + const onboardingCall = calls.find((call) => + call.path.endsWith("/chat/workbenches/chan-1/onboarding"), + ); + expect(JSON.parse(String(onboardingCall?.init?.body))).toEqual({ + kind: "connect-github", + requiredForTemplate: "Code review", + promise: CODE_REVIEW_TEMPLATE.promise, + steps: CODE_REVIEW_TEMPLATE.onboardingSteps.map(({ title, why }) => ({ + title, + why, + })), + }); + + const createBodyParsed = JSON.parse(String(createCall?.init?.body)); + expect(createBodyParsed.kind).toBe("workbench"); + expect(createBodyParsed.definitionId).toBeUndefined(); expect(navigated).toEqual(["/w/chan-1"]); // CL-6594: a room this function navigates to must never carry a @@ -293,7 +319,6 @@ describe("createWorkbenchFromTemplate (CL-6387)", () => { "blank", (to) => navigated.push(to), newQueryClient(), - undefined, "Plan the Q3 launch", ); @@ -315,7 +340,7 @@ describe("createWorkbenchFromTemplate (CL-6387)", () => { if (path.endsWith("/library/templates/code-review")) { return json({ id: "code-review", - content: serializeWorkbenchTemplateManifest(CODE_REVIEW_TEMPLATE), + content: serializeWorkbenchDefinition(CODE_REVIEW_TEMPLATE), }); } if (path.endsWith("/chat/workbenches")) { @@ -333,6 +358,9 @@ describe("createWorkbenchFromTemplate (CL-6387)", () => { if (path.endsWith("/agent-definitions")) { return json({ ...assistantDefinitionWire, id: "def-reviewer-1" }); } + if (path.endsWith("/chat/workbenches/chan-1/onboarding")) { + return json({ id: "msg-onboarding" }, 201); + } if (path.endsWith("/chat/workbenches/chan-1/invite")) { return json({ address: "agent:invited", @@ -366,7 +394,6 @@ describe("createWorkbenchFromTemplate (CL-6387)", () => { "code-review", () => {}, newQueryClient(), - undefined, "Review the auth PR", ); @@ -383,4 +410,89 @@ describe("createWorkbenchFromTemplate (CL-6387)", () => { ), ).toBe(false); }); + + // GitHub already connected is not a different create path: the in-room + // card reads live connected state and flips itself to repo pick, so the + // create flow posts the same walkthrough card and never picks repos or + // starts reviewing on the person's behalf. + test("with GitHub already connected, create posts the same walkthrough and never starts reviewing itself", async () => { + 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: serializeWorkbenchDefinition(CODE_REVIEW_TEMPLATE), + }); + } + if (path.endsWith("/chat/workbenches")) { + return json({ + id: "chan-1", + title: "Code review", + kind: "workbench", + 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")) { + return json({ ...assistantDefinitionWire, id: "def-reviewer-1" }); + } + if (path.endsWith("/chat/workbenches/chan-1/onboarding")) { + return json({ id: "msg-onboarding" }, 201); + } + 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: "workbench", + pinned: false, + participants: [], + settings: {}, + contextWindow: { value: 0, source: "inherit" }, + }); + } + if (path.includes("/credentials/resolve/")) { + return json({ + id: "cred_github", + tenantId: "tnt_1", + name: "GitHub", + status: "active", + }); + } + throw new Error(`unexpected fetch: ${path}`); + }); + + await createWorkbenchFromTemplate( + "tnt_1", + "code-review", + () => undefined, + newQueryClient(), + ); + + const onboardingCall = calls.find((call) => + call.path.endsWith("/chat/workbenches/chan-1/onboarding"), + ); + expect(JSON.parse(String(onboardingCall?.init?.body))).toEqual({ + kind: "connect-github", + requiredForTemplate: "Code review", + promise: CODE_REVIEW_TEMPLATE.promise, + steps: CODE_REVIEW_TEMPLATE.onboardingSteps.map(({ title, why }) => ({ + title, + why, + })), + }); + expect(calls.some((call) => call.path.includes("/github/state"))).toBe( + false, + ); + expect( + calls.some((call) => call.path.includes("/github/start-reviewing")), + ).toBe(false); + }); }); diff --git a/apps/web/src/instant-agent-create.ts b/apps/web/src/instant-agent-create.ts index 9355225bb..53ad80639 100644 --- a/apps/web/src/instant-agent-create.ts +++ b/apps/web/src/instant-agent-create.ts @@ -1,32 +1,30 @@ // Every "create a workbench" affordance — the sidebar's "+", the command -// palette's "New workbench", and the zero-workbench land-hop on `/` -// (CL-6486, superseding CL-6138's silent auto-mint) — opens the template -// picker (`pages/new-workbench-picker.tsx`, CL-6342) and calls +// palette's "New workbench", and the zero-workbench land-hop on `/` — +// opens the template picker (`pages/new-workbench-picker.tsx`) and calls // `createWorkbenchFromTemplate` below once a row is chosen. Blank `+` // mints an empty `kind: "workbench"` channel (no host, no definitionId). -// Named templates mint that same empty channel, then invite existing -// principals — including Myra — so the room is a multi-principal -// channel, never a second agent DM. Explicitly defining a brand-new +// A named workbench definition mints that same empty channel, then +// instantiates what the definition describes — its agents, its block +// workflows, its pending plugins — and runs the definition's own +// onboarding walkthrough in the room. Explicitly defining a brand-new // agent, with its own name/purpose/model/skills chosen up front, stays // `CreateAgentPanel`'s job (Settings → Agents), unchanged. -import { getLogger } from "@corbits/client-log"; import type { QueryClient } from "@tanstack/react-query"; import { createWorkbench, - getConnectGithubState, inviteAgent, partsForSend, patchWorkbenchSettings, + postWorkbenchOnboardingStep, sendMessage, - startReviewingGithubRepos, workbenchesQueryKeyPrefix, - type ConnectGithubRepo, } from "@corbits/chat-ui"; -import { listPluginsForTenant } from "@workbench/connections/plugins"; import { instantiateWorkbenchTemplate, templateSettingsPatch, + type WorkbenchDefinition, + type WorkbenchOnboardingStep, } from "@corbits/workflow-catalog"; import { @@ -42,7 +40,6 @@ import { import { findMyraDefinition } from "./myra-workbench"; import { workbenchPath } from "./workbench-path"; -const log = getLogger("web.instant-agent-create"); import type { WorkbenchTemplateId } from "./workbench-templates"; export { NEW_WORKBENCH_TITLE }; @@ -86,39 +83,57 @@ const SETUP_AGENT_MISSING_MESSAGE = "Your workbench is still finishing setup. Try again in a moment."; /** - * Presents the connected org's repo list for the person to pick from once - * the workbench exists — the create flow's own "select" half of CL-6386 - * ("connect in Plugins; select on new-workbench"). Resolving `null` means - * "skip" (the person closed the picker without choosing); an empty array - * is a legitimate "review nothing yet" choice, distinct from skipping. + * Raises the walkthrough's first card in the room. Only a + * `connect-plugin github` step has an in-room card today; the steps + * after it (`pick-github-repos`, `start-webhook-trigger`) are driven by + * that same card as the person works through it, so they need no client + * action here. The whole ordered walkthrough rides along in the card's + * body, so its step rail renders the definition's own copy. */ -export type PickGithubRepos = (args: { - readonly orgName: string; - readonly repos: readonly ConnectGithubRepo[]; - readonly selectedRepoIds: readonly string[]; -}) => Promise; +async function postOnboardingWalkthrough( + tenantId: string, + workbenchId: string, + definition: WorkbenchDefinition, + steps: readonly WorkbenchOnboardingStep[], +): Promise { + const labels = steps.map(({ title, why }) => ({ title, why })); + for (const step of steps) { + if (step.kind !== "connect-plugin") continue; + if (step.connectorId !== "github") { + throw new Error( + `workbench template "${definition.id}" asks to connect ` + + `"${step.connectorId}", which has no in-room onboarding card yet`, + ); + } + await postWorkbenchOnboardingStep(tenantId, workbenchId, { + kind: "connect-github", + requiredForTemplate: definition.title, + promise: definition.promise, + steps: labels, + }); + } +} /** - * The template picker's "Create workbench" action (CL-6344 / CL-6982): - * mints an empty `kind: "workbench"` channel with no host and no - * `definitionId`. A named template (code-review, due-diligence, GTM) - * still mints that empty channel, then instantiates its roster — - * existing principals, including Myra, are invited after mint rather - * than skipped as "already hosted." Talking to an agent is clicking - * that agent (find-or-reopen its one DM). This function is the create - * verb for a room, not a clone of Myra. + * The template picker's "Create workbench" action: mints an empty + * `kind: "workbench"` channel with no host and no `definitionId`, then + * instantiates the picked definition into it — its agents (existing + * principals invited after mint, new ones created first), its block + * workflows, its pending plugins — and posts the first step of the + * definition's onboarding walkthrough as a card in the room. Talking to + * an agent is clicking that agent (find-or-reopen its one DM); this + * function is the create verb for a room. * - * Setup still has to have seeded the default assistant definition; if - * it hasn't, we fail with `WorkbenchPreconditionError` rather than - * minting a hostless room that then can't invite anyone. A template id - * with no manifest yet (`blank`, "Just start talking") mints a plain - * untagged channel under the generic `NEW_WORKBENCH_TITLE`. 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. + * The bench has to be past setup — its default assistant definition + * deployed — before a room can invite anyone into it, so a bench that + * isn't fails with `WorkbenchPreconditionError` rather than minting a + * room nobody can join. That definition is the readiness gate only: no + * definition names it as a host, and none invites it. A template id + * with no definition (`blank`, "Just start talking") mints a plain + * untagged channel under the generic `NEW_WORKBENCH_TITLE`. * - * `queryClient` invalidates the workbenches list once every template - * participant has been invited (CL-6594) — `ChatWorkspace`'s own + * `queryClient` invalidates the workbenches list once every agent the + * definition names has been invited — `ChatWorkspace`'s own * in-room "Invite agent" dialog does the same * (`workbenchesQueryKeyPrefix`, `chat-workspace.tsx`'s * `refreshWorkbenchLists`) so the room the invite landed in never @@ -127,12 +142,11 @@ export type PickGithubRepos = (args: { * holding a `workbenches` query cached from before the last invite * resolved. * - * `firstMessage`, when given (CL-6628's prompt box), is sent as the - * signed-in person's own opening message once the room and its - * template participants exist, so it lands after the setup/template - * greeting rather than racing it — Myra reads the room's actual intent - * as the next line, not the first. For a blank / ad-hoc mint (CL-6656) - * that same text also renames the room off `NEW_WORKBENCH_TITLE` via + * `firstMessage`, when given (the picker's prompt box), is sent as the + * signed-in person's own opening message once the room and the + * definition's agents exist, so it lands after the onboarding card + * rather than racing it. For a blank / ad-hoc mint that same text also + * renames the room off `NEW_WORKBENCH_TITLE` via * `patchWorkbenchSettings` (`chat/name`), matching the sidebar rename * path; prefab titles are left alone. */ @@ -141,7 +155,6 @@ export async function createWorkbenchFromTemplate( templateId: WorkbenchTemplateId, navigate: (to: string) => void, queryClient: QueryClient, - pickGithubRepos?: PickGithubRepos, firstMessage?: string, ): Promise { const definitions = await listAgentDefinitions(tenantId); @@ -152,61 +165,31 @@ export async function createWorkbenchFromTemplate( "setup-agent-missing", ); } - // The manifest comes from the bench library (CL-6344), never from a - // hardcoded catalog import; reading it is what seeds the shelf - // (CL-6458). `blank` is the one id with no manifest by design; any + // The definition comes from the bench library, never from a + // hardcoded catalog import; reading it is what seeds the shelf. + // `blank` is the one id with no definition by design; any // other id resolving to nothing means this build ships no such // template — fail loud rather than mint a workbench missing its // agents. The picker only offers ids the library listed, so this is // the race-loser's message, not the everyday path. - const manifest = + const definition = templateId === "blank" ? undefined : ((await fetchWorkbenchTemplateManifest(tenantId, templateId)) ?? undefined); - if (templateId !== "blank" && manifest === undefined) { + if (templateId !== "blank" && definition === undefined) { throw new WorkbenchPreconditionError( `A ${templateId} workbench isn't available here yet.`, "template-unavailable", ); } - const requiresGithub = - 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. - const githubAlreadyConnected = - requiresGithub && pickGithubRepos !== undefined - ? (await listPluginsForTenant(tenantId)).some( - (plugin) => - plugin.descriptor.id === "github" && plugin.status === "connected", - ) - : false; - const workbench = await createWorkbench(tenantId, { kind: "workbench", - name: manifest?.title ?? NEW_WORKBENCH_TITLE, + name: definition?.title ?? NEW_WORKBENCH_TITLE, }); - if (githubAlreadyConnected && pickGithubRepos !== undefined) { - const state = await getConnectGithubState(tenantId, workbench.id); - if (state.kind === "connected" && state.repos.length > 0) { - const repoIds = await pickGithubRepos({ - orgName: state.orgName, - repos: state.repos, - selectedRepoIds: state.selectedRepoIds, - }); - if (repoIds !== null && repoIds.length > 0) { - await startReviewingGithubRepos(tenantId, workbench.id, repoIds); - } - } - } - - if (manifest !== undefined) { - const result = await instantiateWorkbenchTemplate(manifest, { + if (definition !== undefined) { + await instantiateWorkbenchTemplate(definition, { async listAgentHandles() { const current = await listAgentDefinitions(tenantId); return current.map((definition) => ({ @@ -228,16 +211,18 @@ export async function createWorkbenchFromTemplate( await patchWorkbenchSettings( tenantId, workbench.id, - templateSettingsPatch(manifest.id, pendingConnections), + templateSettingsPatch(definition.id, pendingConnections), + ); + }, + async beginOnboarding(steps) { + await postOnboardingWalkthrough( + tenantId, + workbench.id, + definition, + steps, ); }, }); - // Honest setup-gap notes, not silent stubs — see - // `instantiateWorkbenchTemplate`'s own doc on what these mean and why - // no live webhook trigger exists yet. - for (const todo of result.webhookTriggerTodos) { - log.error(todo); - } await queryClient.invalidateQueries({ queryKey: workbenchesQueryKeyPrefix(tenantId), }); @@ -245,10 +230,10 @@ export async function createWorkbenchFromTemplate( if (firstMessage !== undefined && firstMessage.trim() !== "") { await sendMessage(tenantId, workbench.id, partsForSend(firstMessage, [])); - // CL-6656: blank / ad-hoc mints stay "New Workbench" until named. When the + // Blank / ad-hoc mints stay "New Workbench" until named. When the // prompt box already supplied the opening message, rename via the same // `chat/name` settings PATCH the sidebar rename uses — prefab titles - // (`manifest?.title`) are left alone by `autoNameFromFirstMessage`. + // (`definition?.title`) are left alone by `autoNameFromFirstMessage`. const autoTitle = autoNameFromFirstMessage(workbench.title, firstMessage); if (autoTitle !== undefined) { await patchWorkbenchSettings(tenantId, workbench.id, { diff --git a/apps/web/src/pages/github-repo-select-dialog.tsx b/apps/web/src/pages/github-repo-select-dialog.tsx deleted file mode 100644 index f86c9a221..000000000 --- a/apps/web/src/pages/github-repo-select-dialog.tsx +++ /dev/null @@ -1,72 +0,0 @@ -// CL-6386's "select on new-workbench" half: once GitHub is already -// connected (established from the Plugins page), the create flow reuses -// this dialog to let a person choose which repos this workbench can work -// on, right after the workbench is minted. It wires `ConnectGithubBlockView` -// wholesale — the same connected-state repo checklist the in-room -// connect-github card already renders — rather than forking its layout, -// so there is exactly one "pick repos" UI in this repo. - -import { useState } from "react"; -import { - Dialog, - DialogBody, - DialogContent, - DialogHeader, - DialogTitle, -} from "@corbits/react-ui"; -import { - ConnectGithubBlockView, - type ConnectGithubRepo, -} from "@corbits/chat-ui"; - -export function GithubRepoSelectDialog({ - orgName, - repos, - initialSelectedRepoIds, - onStartReviewing, - onSkip, -}: { - readonly orgName: string; - readonly repos: readonly ConnectGithubRepo[]; - readonly initialSelectedRepoIds: readonly string[]; - readonly onStartReviewing: (repoIds: readonly string[]) => void; - readonly onSkip: () => void; -}) { - const [selectedRepoIds, setSelectedRepoIds] = useState( - initialSelectedRepoIds, - ); - - return ( - { - if (!next) onSkip(); - }} - > - - - Choose repos this workbench can work on - - - - setSelectedRepoIds((current) => - current.includes(repoId) - ? current.filter((id) => id !== repoId) - : [...current, repoId], - ) - } - onSelectAll={() => setSelectedRepoIds(repos.map((repo) => repo.id))} - onChangeConnection={onSkip} - onStartReviewing={onStartReviewing} - onSkip={onSkip} - /> - - - - ); -} diff --git a/apps/web/src/pages/new-workbench-picker.tsx b/apps/web/src/pages/new-workbench-picker.tsx index 08d940f78..cb625a2a4 100644 --- a/apps/web/src/pages/new-workbench-picker.tsx +++ b/apps/web/src/pages/new-workbench-picker.tsx @@ -26,15 +26,12 @@ import { useQueryClient } from "@tanstack/react-query"; import { getLogger } from "@corbits/client-log"; import { ApiQueryError, describeApiError } from "@corbits/api-query"; -import type { ConnectGithubRepo } from "@corbits/chat-ui"; - import { useAPIQuery } from "../api"; import { TemplateLibraryPage } from "../workbench-templates-api"; import { useBench } from "../bench-context"; import { createWorkbenchFromTemplate, WorkbenchPreconditionError, - type PickGithubRepos, } from "../instant-agent-create"; import { fetchAgentReadiness } from "../onboarding"; import { useNavigate } from "../navigation"; @@ -43,7 +40,6 @@ import { WORKBENCH_TEMPLATES, type WorkbenchTemplateId, } from "../workbench-templates"; -import { GithubRepoSelectDialog } from "./github-repo-select-dialog"; const log = getLogger("web.new-workbench-picker"); @@ -72,13 +68,6 @@ export function describeWorkbenchCreateFailure(cause: unknown): string { return GENERIC_CREATE_FAILURE; } -type RepoPickerState = { - readonly orgName: string; - readonly repos: readonly ConnectGithubRepo[]; - readonly selectedRepoIds: readonly string[]; - readonly resolve: (repoIds: readonly string[] | null) => void; -}; - const CARD_ICON: Record = { "code-review": GitPullRequest, "due-diligence": MagnifyingGlass, @@ -110,7 +99,6 @@ export function NewWorkbenchPickerRoute() { // Distinct from `creating`'s loader: this is a dead end until setup // finishes, not a request in flight. const [stillSettingUp, setStillSettingUp] = useState(false); - const [repoPicker, setRepoPicker] = useState(null); const promptRef = useRef(null); // The last attempted create, so "Try again" (both the still-setting-up // dead end and a plain toast-and-retry) replays the exact same request @@ -146,15 +134,6 @@ export function NewWorkbenchPickerRoute() { (template) => !offeredTemplates.includes(template), ); - const pickGithubRepos: PickGithubRepos = ({ - orgName, - repos, - selectedRepoIds, - }) => - new Promise((resolve) => { - setRepoPicker({ orgName, repos, selectedRepoIds, resolve }); - }); - async function handleCreate( templateId: WorkbenchTemplateId, firstMessage?: string, @@ -169,7 +148,6 @@ export function NewWorkbenchPickerRoute() { templateId, navigate, queryClient, - pickGithubRepos, firstMessage, ); } catch (cause) { @@ -365,21 +343,6 @@ export function NewWorkbenchPickerRoute() { )} - {repoPicker !== null ? ( - { - repoPicker.resolve(repoIds); - setRepoPicker(null); - }} - onSkip={() => { - repoPicker.resolve(null); - setRepoPicker(null); - }} - /> - ) : null} ); } diff --git a/apps/web/src/workbench-templates-api.ts b/apps/web/src/workbench-templates-api.ts index 5f26b4493..dc7abceb7 100644 --- a/apps/web/src/workbench-templates-api.ts +++ b/apps/web/src/workbench-templates-api.ts @@ -3,14 +3,14 @@ // from a hardcoded `@corbits/workflow-catalog` import. The read itself // is what converges the shelf (CL-6458), so these routes answer for a // bench of any age. The wire shape is the library's `{id, content}` -// entry; the content string re-enters through the catalog's own manifest -// schema, so a corrupt or stale library row fails loud here rather than -// half-instantiating a workbench. +// entry; the content string re-enters through the catalog's own +// `WorkbenchDefinition` schema, so a corrupt or stale library row fails +// loud here rather than half-instantiating a workbench. import { type } from "arktype"; import { - parseWorkbenchTemplateManifest, - type WorkbenchTemplateManifest, + parseWorkbenchDefinition, + type WorkbenchDefinition, } from "@corbits/workflow-catalog"; import { ApiQueryError } from "@corbits/api-query"; @@ -79,7 +79,7 @@ export async function deployWorkbenchTemplateBlock( export async function fetchWorkbenchTemplateManifest( tenantId: string, templateId: string, -): Promise { +): Promise { const path = `/api/tenants/${tenantId}/library/templates/${templateId}`; let response: Response; try { @@ -109,5 +109,5 @@ export async function fetchWorkbenchTemplateManifest( path, ); } - return parseWorkbenchTemplateManifest(entry.content); + return parseWorkbenchDefinition(entry.content); } diff --git a/apps/web/test/new-workbench-picker.test.tsx b/apps/web/test/new-workbench-picker.test.tsx index c1ba6ed5e..00345ef4b 100644 --- a/apps/web/test/new-workbench-picker.test.tsx +++ b/apps/web/test/new-workbench-picker.test.tsx @@ -11,7 +11,7 @@ import { afterEach, describe, expect, test } from "bun:test"; import { CODE_REVIEW_TEMPLATE, DUE_DILIGENCE_TEMPLATE, - serializeWorkbenchTemplateManifest, + serializeWorkbenchDefinition, } from "@corbits/workflow-catalog"; import { act } from "react"; import { createRoot, type Root } from "react-dom/client"; @@ -70,7 +70,7 @@ function stubFetch( data: [ { id: "code-review", - content: serializeWorkbenchTemplateManifest(CODE_REVIEW_TEMPLATE), + content: serializeWorkbenchDefinition(CODE_REVIEW_TEMPLATE), }, ], }), @@ -80,7 +80,7 @@ function stubFetch( return Promise.resolve( json({ id: "code-review", - content: serializeWorkbenchTemplateManifest(CODE_REVIEW_TEMPLATE), + content: serializeWorkbenchDefinition(CODE_REVIEW_TEMPLATE), }), ); } @@ -264,14 +264,11 @@ describe("NewWorkbenchPickerRoute", () => { data: [ { id: "code-review", - content: - serializeWorkbenchTemplateManifest(CODE_REVIEW_TEMPLATE), + content: serializeWorkbenchDefinition(CODE_REVIEW_TEMPLATE), }, { id: "due-diligence", - content: serializeWorkbenchTemplateManifest( - DUE_DILIGENCE_TEMPLATE, - ), + content: serializeWorkbenchDefinition(DUE_DILIGENCE_TEMPLATE), }, ], }), @@ -516,6 +513,12 @@ describe("NewWorkbenchPickerRoute", () => { definitionId: body.definitionId, }); } + if ( + path.endsWith("/chat/workbenches/chan_new/onboarding") && + init?.method === "POST" + ) { + return json({ id: "msg_onboarding" }, 201); + } if (path.endsWith("/chat/workbenches/chan_new/settings")) { return json({ id: "chan_new", @@ -530,14 +533,6 @@ describe("NewWorkbenchPickerRoute", () => { contextWindow: { value: 0, source: "inherit" }, }); } - // The create flow checks whether GitHub is already connected - // (CL-6386's "select on new-workbench" half) before deciding - // whether to post the in-room card or go straight to repo - // selection — nothing is connected in this fixture, so every - // connector resolves 404/not-found. - if (path.includes("/credentials/resolve/")) { - return json({ error: "not_found" }, 404); - } return undefined; }); @@ -592,8 +587,10 @@ describe("NewWorkbenchPickerRoute", () => { }); }); - test("with GitHub already connected, clicking Code review skips the in-room card and mints grants from an inline repo pick (CL-6386)", async () => { - const calls = stubFetch((path, init) => { + // The in-room card is the one walkthrough: /new never opens a repo + // dialog of its own, connected or not. + test("clicking the Code review card opens no repo dialog — the walkthrough card owns repo pick", async () => { + stubFetch((path, init) => { if (path.includes("/workflows/definitions")) { return json({ data: [ @@ -650,6 +647,12 @@ describe("NewWorkbenchPickerRoute", () => { definitionId: body.definitionId, }); } + if ( + path.endsWith("/chat/workbenches/chan_new/onboarding") && + init?.method === "POST" + ) { + return json({ id: "msg_onboarding" }, 201); + } if (path.endsWith("/chat/workbenches/chan_new/settings")) { return json({ id: "chan_new", @@ -664,43 +667,6 @@ describe("NewWorkbenchPickerRoute", () => { contextWindow: { value: 0, source: "inherit" }, }); } - // GitHub already connected at the tenant level. - if (path.includes("/credentials/resolve/GitHub")) { - return json({ - id: "cred_github", - tenantId: "tnt_1", - name: "GitHub", - status: "active", - }); - } - if (path.includes("/credentials/resolve/")) { - return json({ error: "not_found" }, 404); - } - if (path.endsWith("/workbenches/chan_new/github/state")) { - return json({ - kind: "connected", - orgName: "acme", - repos: [ - { - id: "repo_widgets", - name: "acme/widgets", - openPullRequestCount: 2, - }, - { - id: "repo_sprockets", - name: "acme/sprockets", - openPullRequestCount: 0, - }, - ], - selectedRepoIds: [], - }); - } - if ( - path.endsWith("/workbenches/chan_new/github/start-reviewing") && - init?.method === "POST" - ) { - return json({ startedTriggerCount: 1 }); - } return undefined; }); @@ -713,44 +679,19 @@ describe("NewWorkbenchPickerRoute", () => { await act(async () => { codeReview?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); - - let startReviewingButton: HTMLButtonElement | undefined; - for (let i = 0; i < 20; i++) { - await settle(); - startReviewingButton = Array.from( - document.querySelectorAll("button"), - ).find((button) => button.textContent?.startsWith("Start reviewing")); - if (startReviewingButton !== undefined) break; - } - expect(startReviewingButton).not.toBeUndefined(); - expect(document.body.textContent).toContain( - "Choose repos this workbench can work on", - ); - - const selectAllButton = Array.from( - document.querySelectorAll("button"), - ).find((button) => button.textContent === "Select all"); - await act(async () => { - selectAllButton?.dispatchEvent( - new MouseEvent("click", { bubbles: true }), - ); - }); - - await act(async () => { - startReviewingButton?.dispatchEvent( - new MouseEvent("click", { bubbles: true }), - ); - }); for (let i = 0; i < 20; i++) { await settle(); if (navigated.length > 0) break; } expect(navigated).toEqual(["/w/chan_new"]); - - const startReviewingCall = calls.find((call) => - call.path.endsWith("/workbenches/chan_new/github/start-reviewing"), + expect(document.body.textContent).not.toContain( + "Choose repos this workbench can work on", ); - expect(startReviewingCall).not.toBeUndefined(); + expect( + Array.from(document.querySelectorAll("button")).some((button) => + button.textContent?.startsWith("Start reviewing"), + ), + ).toBe(false); }); }); diff --git a/packages/evals/src/targets/real-target.ts b/packages/evals/src/targets/real-target.ts index de64dcbfc..1e172e528 100644 --- a/packages/evals/src/targets/real-target.ts +++ b/packages/evals/src/targets/real-target.ts @@ -31,7 +31,7 @@ import { completeCredentialSetup } from "@workbench/onboarding"; import { OLLAMA_PLACEHOLDER_SECRET } from "@workbench/hub-client"; import { instantiateWorkbenchTemplate, - parseWorkbenchTemplateManifest, + parseWorkbenchDefinition, templateSettingsPatch, } from "@corbits/workflow-catalog"; import { @@ -679,7 +679,7 @@ export async function bootMyraTarget( // The REAL install path (#140): the exact surfaces // `apps/web/src/instant-agent-create.ts`'s - // `createWorkbenchFromTemplate` drives — seeded-library manifest + // `createWorkbenchFromTemplate` drives — seeded-library definition // read, workbench mint, `instantiateWorkbenchTemplate` over // HTTP-bound ports — never an eval-only instantiation mechanism. // The library read is also what seeds this scratch tenant's shelf @@ -693,14 +693,15 @@ export async function bootMyraTarget( cookies, ); expectStatus(`fetch seeded template "${templateId}"`, entryRes, 200); - const manifest = parseWorkbenchTemplateManifest( + const definition = parseWorkbenchDefinition( stringField(entryRes.data, "content", `template "${templateId}"`), ); + // Hostless, exactly as the web create flow mints it: a template + // room has no `definitionId` and nobody is hosted in it. const createBody: Record = { - kind: "chat", - definitionId: assistantDefinitionId, - name: "New Workbench", + kind: "workbench", + name: definition.title, }; const createRes = await api( hub.baseUrl, @@ -716,7 +717,7 @@ export async function bootMyraTarget( `create workbench from "${templateId}"`, ); - const result = await instantiateWorkbenchTemplate(manifest, { + const result = await instantiateWorkbenchTemplate(definition, { async listAgentHandles() { const res = await api( hub.baseUrl, @@ -787,21 +788,38 @@ export async function bootMyraTarget( hub.baseUrl, "PATCH", `/api/tenants/${seeded.tenantId}/chat/workbenches/${workbenchId}/settings`, - templateSettingsPatch(manifest.id, pendingConnections), + templateSettingsPatch(definition.id, pendingConnections), cookies, ); expectStatus("record pending connections", res, 200); }, + async beginOnboarding(steps) { + for (const step of steps) { + if (step.kind !== "connect-plugin") continue; + const res = await api( + hub.baseUrl, + "POST", + `/api/tenants/${seeded.tenantId}/chat/workbenches/${workbenchId}/onboarding`, + { + kind: "connect-github", + requiredForTemplate: definition.title, + promise: definition.promise, + steps: steps.map(({ title, why }) => ({ title, why })), + }, + cookies, + ); + expectStatus("post the onboarding walkthrough card", res, 201); + } + }, }); - // The repo-selection half of the create flow (CL-6386's "select on - // new-workbench", the same sequence `createWorkbenchFromTemplate` - // drives when GitHub is already connected): read the connect - // card's live state, then start reviewing every listed repo — + // The rest of the definition's onboarding walkthrough, driven + // here rather than by a person clicking the in-room card: read the + // connect card's live state, then start reviewing every listed repo — // which mints the per-repo grant and `webhook_trigger` row the // fire-webhook step needs. let startedTriggerCount = 0; - if (manifest.requiredConnections.includes("github")) { + if (definition.plugins.required.includes("github")) { const stateRes = await api( hub.baseUrl, "GET", diff --git a/packages/workflow-catalog/README.md b/packages/workflow-catalog/README.md index 256c21aaa..5f71cbbcd 100644 --- a/packages/workflow-catalog/README.md +++ b/packages/workflow-catalog/README.md @@ -37,14 +37,18 @@ trigger fields reference). `isAutomatableWorkflowName`, `deliveryChannelRequiredForWorkflowName`, `workflowCatalogEntry`, `workflowDisplayName`, and `validateTriggerFieldsInput`. -- `src/templates.ts` — `WorkbenchTemplateManifest` and the templates - themselves (`GTM_TEMPLATE`, `CODE_REVIEW_TEMPLATE`), each validated at +- `src/templates.ts` — `WorkbenchDefinition` (default agents, routines, + tools, plugins, and the ordered `onboardingSteps` walkthrough) and the + shipped definitions themselves (`GTM_TEMPLATE`, + `CODE_REVIEW_TEMPLATE`, `DUE_DILIGENCE_TEMPLATE`), each validated at module load; `workbenchTemplate(id)` looks one up. - `src/instantiate.ts` — `instantiateWorkbenchTemplate`: resolves a - manifest against a bench over injected ports (create the participant - agent definitions that don't exist yet, record required connections as - pending). Today this only supports a manifest whose participants are - backed by `@corbits/code-review`'s reviewer roster. + definition against a bench over injected ports (create the agent + definitions that don't exist yet, record required plugins as pending, + then begin the definition's onboarding walkthrough). Today this only + supports a definition whose agents are backed by + `@corbits/code-review`'s reviewer roster or a standalone chat agent + this package can build a create request for. - `src/settings.ts` — the `template/*` workbench-settings vocabulary a template-instantiated room persists about itself. diff --git a/packages/workflow-catalog/src/index.ts b/packages/workflow-catalog/src/index.ts index a1a9e678d..27bf230bc 100644 --- a/packages/workflow-catalog/src/index.ts +++ b/packages/workflow-catalog/src/index.ts @@ -13,19 +13,20 @@ export { DUE_DILIGENCE_TEMPLATE, GTM_TEMPLATE, WORKBENCH_TEMPLATES, + WorkbenchDefinitionAgent, + WorkbenchDefinitionSchema, + WorkbenchOnboardingStep, WorkbenchTemplateBlock, WorkbenchTemplateOpenInput, - WorkbenchTemplateParticipant, WorkbenchTemplateRoutine, WorkbenchTemplateWebhookTrigger, - WorkbenchTemplateManifestSchema, - parseWorkbenchTemplateManifest, - serializeWorkbenchTemplateManifest, + parseWorkbenchDefinition, + serializeWorkbenchDefinition, templateBlockAssetNames, workbenchTemplate, workbenchTemplateLibraryEntries, } from "./templates"; -export type { WorkbenchTemplateManifest } from "./templates"; +export type { WorkbenchDefinition } from "./templates"; export { instantiateWorkbenchTemplate, type ParticipantAgentRequest, diff --git a/packages/workflow-catalog/src/instantiate.ts b/packages/workflow-catalog/src/instantiate.ts index e99753785..7370a3bcd 100644 --- a/packages/workflow-catalog/src/instantiate.ts +++ b/packages/workflow-catalog/src/instantiate.ts @@ -1,27 +1,27 @@ -// Turns a picked template into the state a freshly minted workbench -// needs: the participant agent definitions that don't already exist, -// and the required-connections list the room persists so the inline -// connect card (CL-6344's next slice) knows what to ask for. Pure -// orchestration over injected ports — no HTTP, no store — so a host -// (today, `apps/web`'s `instant-agent-create.ts`) can bind the ports to -// its own REST clients and this stays testable with plain fakes. +// Turns a picked workbench definition into the state a freshly minted +// workbench needs: the agent definitions that don't already exist, the +// block workflows behind them, the still-pending plugin list the room +// persists, and the definition's ordered onboarding walkthrough handed +// to whatever surface runs it. Pure orchestration over injected ports — +// no HTTP, no store — so a host (today, `apps/web`'s +// `instant-agent-create.ts`) can bind the ports to its own REST clients +// and this stays testable with plain fakes. // -// This resolves a manifest whose non-Myra participants are backed by -// either `@corbits/code-review`'s reviewer roster (CL-6344's -// `CODE_REVIEW_TEMPLATE`) or a standalone chat agent this catalog installs -// the same way — Scout, for `DUE_DILIGENCE_TEMPLATE` (see -// `./participant-agent-requests.ts`). Jimmy resolves through the identical -// `ParticipantAgentRequest` shape (`jimmyAgentRequest()`), but CL-6499 -// dropped his template — he is not a "kind of workbench" — so no shipped -// manifest names his handle today; `@corbits/chat-ui`'s "Add Jimmy" -// quick-create row calls `jimmyAgentRequest()` directly instead. Kept -// registered here too so a future template naming his handle resolves -// without new plumbing. -// A template like `GTM_TEMPLATE`, whose participants are backed by their -// own deployed workflow definitions rather than an agent-directory -// create request, needs its own resolution path — a later ticket, not -// this one; calling this function against such a manifest throws rather -// than silently doing nothing. +// This resolves a definition whose non-Myra agents are backed by either +// `@corbits/code-review`'s reviewer roster (`CODE_REVIEW_TEMPLATE`) or a +// standalone chat agent this catalog installs the same way — Scout, for +// `DUE_DILIGENCE_TEMPLATE` (see `./participant-agent-requests.ts`). +// Jimmy resolves through the identical `ParticipantAgentRequest` shape +// (`jimmyAgentRequest()`), but no shipped definition names his handle +// today — he is not a "kind of workbench"; `@corbits/chat-ui`'s "Add +// Jimmy" quick-create row calls `jimmyAgentRequest()` directly instead. +// Kept registered here too so a future definition naming his handle +// resolves without new plumbing. +// +// A definition like `GTM_TEMPLATE`, whose agents are backed by their own +// deployed workflow definitions rather than an agent-directory create +// request, needs its own resolution path — calling this function against +// such a definition throws rather than silently doing nothing. import { codeReviewAgentRequests, type CodeReviewAgentRequest, @@ -32,13 +32,14 @@ import { } from "./participant-agent-requests"; import type { + WorkbenchDefinition, + WorkbenchOnboardingStep, WorkbenchTemplateBlock, - WorkbenchTemplateManifest, } from "./templates"; -/** The agent-directory create-request shape every participant resolves +/** The agent-directory create-request shape every agent resolves * to: `CodeReviewAgentRequest`'s own fields, plus the tool-package pins - * a tool-calling participant (Scout, Jimmy) needs and a pure-text + * a tool-calling agent (Scout, Jimmy) needs and a pure-text * reviewer does not. */ export type ParticipantAgentRequest = CodeReviewAgentRequest & { readonly toolPackagePins?: readonly string[]; @@ -50,7 +51,7 @@ export interface WorkbenchTemplateInstantiationPorts { * 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 + * already has is not yet a member of a freshly minted room, so * its id still has to reach the invite call even when this function * skips creating it. */ listAgentHandles(): Promise< @@ -61,8 +62,8 @@ export interface WorkbenchTemplateInstantiationPorts { createParticipantAgent( request: ParticipantAgentRequest, ): Promise<{ readonly id: string }>; - /** Deploys one of the manifest's referenced block workflows through - * the same source-form deploy the participant agents use + /** Deploys one of the definition's referenced block workflows through + * the same source-form deploy the agent definitions use * (`POST /template-blocks/:assetName/deploy` — see * `./template-block-routes.ts`), or a fake of it in tests. `created` * is `false` when the tenant already carries a deployed definition @@ -71,10 +72,10 @@ export interface WorkbenchTemplateInstantiationPorts { deployBlockWorkflow( block: WorkbenchTemplateBlock, ): Promise<{ readonly created: boolean }>; - /** Adds one participant's agent definition to the newly created + /** Adds one 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 + * is what makes a definition's roster actually present in the room * rather than merely registered in the agent directory. Called for * Myra too: she is an existing principal invited after mint, not * the room's host `definitionId`. */ @@ -85,15 +86,20 @@ export interface WorkbenchTemplateInstantiationPorts { recordPendingConnections( pendingConnections: readonly string[], ): Promise; + /** Runs the definition's onboarding walkthrough in the freshly minted + * room — `apps/web` posts the first step's card through + * `postWorkbenchOnboardingStep`. Called last, and only when the + * definition has steps at all. */ + beginOnboarding(steps: readonly WorkbenchOnboardingStep[]): Promise; } 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. */ + /** Every non-Myra agent handle actually added to the room — + * `createdHandles` and `skippedHandles` combined, in definition order. + * A caller proving the roster a definition 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 @@ -101,41 +107,23 @@ export interface WorkbenchTemplateInstantiationResult { readonly deployedBlockAssetNames: readonly string[]; readonly skippedBlockAssetNames: readonly string[]; readonly pendingConnections: readonly string[]; - /** - * One line per webhook trigger this template names, honestly stating - * that no live `webhook_trigger` row exists yet — resolved by - * `./connect-github-setup.ts`'s `startReviewingRepos` once the person - * has picked repos on the connect card, not by this function. Never a - * silent stub: a caller surfacing these tells the person setup isn't - * done rather than pretending it is. - */ - readonly webhookTriggerTodos: readonly string[]; -} - -function webhookTriggerTodo( - manifest: WorkbenchTemplateManifest, - trigger: WorkbenchTemplateManifest["webhookTriggers"][number], -): string { - return ( - `pending: the live webhook_trigger row for "${manifest.id}"'s ` + - `"${trigger.key}" trigger is created once the person picks repos on the ` + - "room's GitHub connect card — see `./connect-github-setup.ts`'s " + - "`startReviewingRepos` (CL-6345), which this manifest-resolution step " + - "has no repo to hand off to yet." - ); + /** The ordered walkthrough handed to `beginOnboarding` — empty for a + * definition with no onboarding at all. */ + readonly onboardingSteps: readonly WorkbenchOnboardingStep[]; } /** - * Resolves `manifest` against the bench: creates the participant agent + * Resolves `definition` against the bench: creates the agent * definitions that don't already exist (Myra is never re-created — she * is the bench's seeded default setup agent, reused as-is and invited - * into the new channel), and records the manifest's required - * connections as still pending. Never registers a live webhook trigger - * — see `webhookTriggerTodos` and `./connect-github-setup.ts`, which is - * what actually creates one, once the person has picked repos. + * into the new channel), records the definition's required plugins as + * still pending, and then begins its onboarding walkthrough. Never + * registers a live webhook trigger itself: that is what the + * walkthrough's own start-reviewing step does, once the person has + * picked repos. */ export async function instantiateWorkbenchTemplate( - manifest: WorkbenchTemplateManifest, + definition: WorkbenchDefinition, ports: WorkbenchTemplateInstantiationPorts, ): Promise { const existingIdsByHandle = new Map( @@ -153,14 +141,14 @@ export async function instantiateWorkbenchTemplate( ].map((request) => [request.handle, request]), ); - // The manifest's referenced block workflows deploy first: a - // participant is a lens over a block, and the connect card's + // The definition's referenced block workflows deploy first: an agent + // is a lens over a block, and the connect card's // start-reviewing step resolves the deployed block definition by // name, so a bench must never end up with reviewers but no // `code-review` workflow behind them. const deployedBlockAssetNames: string[] = []; const skippedBlockAssetNames: string[] = []; - for (const block of manifest.blocks) { + for (const block of definition.blocks) { const outcome = await ports.deployBlockWorkflow(block); (outcome.created ? deployedBlockAssetNames : skippedBlockAssetNames).push( block.assetName, @@ -170,34 +158,38 @@ export async function instantiateWorkbenchTemplate( const createdHandles: string[] = []; const skippedHandles: string[] = []; const invitedHandles: string[] = []; - for (const participant of manifest.participants) { - const existingId = existingIdsByHandle.get(participant.handle); - let participantId: string; + for (const agent of definition.agents) { + const existingId = existingIdsByHandle.get(agent.handle); + let agentId: string; if (existingId !== undefined) { - skippedHandles.push(participant.handle); - participantId = existingId; - } else if (participant.handle === "myra") { + skippedHandles.push(agent.handle); + agentId = existingId; + } else if (agent.handle === "myra") { // Seeded principal (`name: "assistant"`), never minted from a - // template roster. The create path already gates on + // definition's roster. The create path already gates on // `findMyraDefinition`; skip rather than throw. continue; } else { - const request = requestsByHandle.get(participant.handle); + const request = requestsByHandle.get(agent.handle); if (request === undefined) { throw new Error( - `workbench template "${manifest.id}" participant "${participant.handle}" ` + + `workbench definition "${definition.id}" agent "${agent.handle}" ` + "has no known create-agent request to instantiate it from", ); } const created = await ports.createParticipantAgent(request); - createdHandles.push(participant.handle); - participantId = created.id; + createdHandles.push(agent.handle); + agentId = created.id; } - await ports.inviteParticipantAgent(participantId); - invitedHandles.push(participant.handle); + await ports.inviteParticipantAgent(agentId); + invitedHandles.push(agent.handle); } - await ports.recordPendingConnections(manifest.requiredConnections); + await ports.recordPendingConnections(definition.plugins.required); + + if (definition.onboardingSteps.length > 0) { + await ports.beginOnboarding(definition.onboardingSteps); + } return { createdHandles, @@ -205,9 +197,7 @@ export async function instantiateWorkbenchTemplate( invitedHandles, deployedBlockAssetNames, skippedBlockAssetNames, - pendingConnections: manifest.requiredConnections, - webhookTriggerTodos: manifest.webhookTriggers.map((trigger) => - webhookTriggerTodo(manifest, trigger), - ), + pendingConnections: definition.plugins.required, + onboardingSteps: definition.onboardingSteps, }; } diff --git a/packages/workflow-catalog/src/templates.ts b/packages/workflow-catalog/src/templates.ts index cc7e4f522..ac0636819 100644 --- a/packages/workflow-catalog/src/templates.ts +++ b/packages/workflow-catalog/src/templates.ts @@ -1,21 +1,25 @@ -// Workbench templates: what "pick a kind of workbench" actually creates. +// Workbench definitions: the single description of what "pick a kind of +// workbench" actually creates — its default agents, routines, tools, +// plugins, and the ordered onboarding walkthrough a person works +// through once the room exists. // // `./index.ts`'s `WORKFLOW_CATALOG` describes ONE workflow at a time — // what it does, what it needs connected, what its trigger carries. A -// template is the layer above: a named workbench worth having, assembled -// out of several of those workflows, the routines that keep it running, -// the agents a person will talk to in it, and the handful of answers -// only they can give. +// definition is the layer above: a named workbench worth having, +// assembled out of several of those workflows. The three shipped +// definitions here are the bench library's templates; a *template* is a +// shipped definition, not a second kind of thing. // -// Everything here is pure data. Creating a workbench from a template is -// a host concern (`apps/web`'s picker starts the flow); this module is -// the single description both the picker and the creator read, so -// neither hand-types an asset name, a cron, or a connector id. +// Everything here is pure data. Creating a workbench from a definition +// is a host concern (`apps/web`'s picker starts the flow); this module +// is the single description both the picker and the creator read, so +// neither hand-types an asset name, a cron, a connector id, or a step +// of onboarding copy. // -// Blocks are referenced by asset name AND version. A template names the -// exact `workflows//package.json` version it was designed against, -// so bumping a workflow is a deliberate edit here rather than a silent -// change in what a template creates. +// Blocks are referenced by asset name AND version. A definition names +// the exact `workflows//package.json` version it was designed +// against, so bumping a workflow is a deliberate edit here rather than +// a silent change in what it creates. import { type } from "arktype"; // Imported from the package's own `./reviewers` subpath, never its root @@ -23,12 +27,13 @@ import { type } from "arktype"; // run and GitHub client, which pull in `@corbits/github-tools` and // `@intx/agent`'s full provider surface. `reviewers.ts` itself has no // imports at all, so this subpath keeps every consumer of this -// manifest (this package's whole point) off that much heavier graph. +// definition (this package's whole point) off that much heavier graph. import { CODE_REVIEW_REVIEWERS } from "@corbits/code-review/reviewers"; import { SCOUT_AGENT_HANDLE, SCOUT_AGENT_DISPLAY_NAME, SCOUT_AGENT_DESCRIPTION, + SCOUT_TOOL_PACKAGE_PINS, } from "@corbits/scout-agent/definition"; /** One workflow a template installs, pinned to the version it was * designed against. `assetName` matches a `WORKFLOW_CATALOG` entry. */ @@ -60,21 +65,19 @@ export type WorkbenchTemplateRoutine = typeof WorkbenchTemplateRoutine.infer; /** * One agent a person can address in the created workbench. `handle` is * what they type to reach it. `blockAssetName` names the workflow behind - * it when the participant is a lens over one of the template's own - * blocks (the code-review reviewers); it is absent for a participant - * that is a standalone chat agent installed straight through the - * agent-directory create path (Scout, Jimmy) with no block of its own to - * reference. + * it when the agent is a lens over one of the definition's own blocks + * (the code-review reviewers); it is absent for an agent that is a + * standalone chat agent installed straight through the agent-directory + * create path (Scout, Jimmy) with no block of its own to reference. */ -export const WorkbenchTemplateParticipant = type({ +export const WorkbenchDefinitionAgent = type({ handle: "/^[a-z][a-z0-9-]*$/", displayName: "string > 0", "blockAssetName?": "string > 0", /** One honest line: what this agent is for. */ role: "string > 0", }); -export type WorkbenchTemplateParticipant = - typeof WorkbenchTemplateParticipant.infer; +export type WorkbenchDefinitionAgent = typeof WorkbenchDefinitionAgent.infer; /** * One workflow a template fires from an inbound webhook rather than a @@ -121,25 +124,49 @@ export type WorkbenchTemplateOpenInput = typeof WorkbenchTemplateOpenInput.infer; /** - * The full manifest, as parsed back off a trust boundary — the bench + * One step in a definition's onboarding walkthrough, in the order a + * person works through it. This is the *definition-level* step — the + * full ordered walkthrough a template describes. `@corbits/chat`'s own + * `WorkbenchOnboardingStep` is the narrower wire-level body a host + * posts into a room to raise one card; the two are different types and + * neither is derived from the other. + */ +export const WorkbenchOnboardingStep = type({ + kind: "'connect-plugin'", + connectorId: "string > 0", + title: "string > 0", + why: "string > 0", +}) + .or({ kind: "'pick-github-repos'", title: "string > 0", why: "string > 0" }) + .or({ + kind: "'start-webhook-trigger'", + webhookTriggerKey: "/^[a-z][a-z0-9-]*$/", + title: "string > 0", + why: "string > 0", + }); +export type WorkbenchOnboardingStep = typeof WorkbenchOnboardingStep.infer; + +/** + * The full definition, as parsed back off a trust boundary — the bench * library row a hub seeded (see `@corbits/artifacts-hub`'s template * library) travels over HTTP before a picker instantiates from it, so * it re-enters through this schema, never through `as`. */ -export const WorkbenchTemplateManifestSchema = type({ +export const WorkbenchDefinitionSchema = type({ id: "/^[a-z][a-z0-9-]*$/", title: "string > 0", promise: "string > 0", blocks: WorkbenchTemplateBlock.array(), - requiredConnections: "string[]", - optionalConnections: "string[]", + plugins: { required: "string[]", optional: "string[]" }, + tools: "string[]", routines: WorkbenchTemplateRoutine.array(), webhookTriggers: WorkbenchTemplateWebhookTrigger.array(), - participants: WorkbenchTemplateParticipant.array(), + agents: WorkbenchDefinitionAgent.array(), openInputs: WorkbenchTemplateOpenInput.array(), + onboardingSteps: WorkbenchOnboardingStep.array(), }); -export type WorkbenchTemplateManifest = { +export type WorkbenchDefinition = { readonly id: string; /** What the picker row calls it. */ readonly title: string; @@ -148,21 +175,27 @@ export type WorkbenchTemplateManifest = { readonly blocks: readonly WorkbenchTemplateBlock[]; /** * Connector ids (see `@workbench/connections`' `CONNECTOR_REGISTRY` and - * `MCP_PRESETS`) this template cannot work without, in the order the - * create flow should ask for them. + * `MCP_PRESETS`): `required` is what this definition cannot work + * without, in the order the walkthrough asks for them; `optional` + * makes it better and never gates the create. */ - readonly requiredConnections: readonly string[]; - /** - * Connectors that make the template better but are not a blocker — the - * create flow offers these, never gates on them. - */ - readonly optionalConnections: readonly string[]; + readonly plugins: { + readonly required: readonly string[]; + readonly optional: readonly string[]; + }; + /** Tool-package names this definition's agents need — package names + * only, never `{name, version}` pins: the pinned version lives with + * the agent package that owns the tool. */ + readonly tools: readonly string[]; readonly routines: readonly WorkbenchTemplateRoutine[]; - /** Webhook-fired triggers this template installs — empty for a - * clock-only template like GTM. */ + /** Webhook-fired triggers this definition installs — empty for a + * clock-only definition like GTM. */ readonly webhookTriggers: readonly WorkbenchTemplateWebhookTrigger[]; - readonly participants: readonly WorkbenchTemplateParticipant[]; + readonly agents: readonly WorkbenchDefinitionAgent[]; readonly openInputs: readonly WorkbenchTemplateOpenInput[]; + /** The ordered walkthrough a freshly created workbench runs — connect + * the plugin, pick what it works on, start the trigger. */ + readonly onboardingSteps: readonly WorkbenchOnboardingStep[]; }; /** @@ -175,7 +208,7 @@ export type WorkbenchTemplateManifest = { * runs on its own weekly clock. The CRM agent and the collateral drafter are * on-demand: both start from a specific thing a person points at. */ -export const GTM_TEMPLATE: WorkbenchTemplateManifest = { +export const GTM_TEMPLATE: WorkbenchDefinition = { id: "gtm", title: "Go to market", promise: @@ -192,8 +225,8 @@ export const GTM_TEMPLATE: WorkbenchTemplateManifest = { // Exa. Granola is what the call backbone runs on — the create flow // offers it up front, but a workbench with the CRM agent and the web // watch alone is still a real workbench, so it never blocks the create. - requiredConnections: ["attio", "exa"], - optionalConnections: ["granola"], + plugins: { required: ["attio", "exa"], optional: ["granola"] }, + tools: [], webhookTriggers: [], routines: [ { @@ -211,7 +244,7 @@ export const GTM_TEMPLATE: WorkbenchTemplateManifest = { why: "One digest at the start of the week, so a quiet week reads as quiet instead of as five empty runs.", }, ], - participants: [ + agents: [ { handle: "crm", displayName: "CRM task agent", @@ -235,18 +268,44 @@ export const GTM_TEMPLATE: WorkbenchTemplateManifest = { appliesToRoutine: "topic-watch", }, ], + // GTM is not offered by the picker yet, and both its instantiation + // paths say so out loud rather than half-creating a workbench: + // `instantiateWorkbenchTemplate` throws because its agents are lenses + // over deployed workflow definitions with no agent-directory create + // request, and the web `beginOnboarding` binding throws because + // neither Attio nor Exa has an in-room onboarding card. + onboardingSteps: [ + { + kind: "connect-plugin", + connectorId: "attio", + title: "Connect Attio", + why: "The CRM agent reads and works your tasks in Attio — without it there is nothing to work.", + }, + { + kind: "connect-plugin", + connectorId: "exa", + title: "Connect Exa", + why: "The weekly web watch reads the open web through Exa.", + }, + { + kind: "connect-plugin", + connectorId: "granola", + title: "Connect Granola", + why: "Connect it and every new call gets written up on its own; skip it and the rest of the workbench still works.", + }, + ], }; /** - * The code-review template (CL-6344): three reviewer lenses over every - * pull request, plus Myra to talk through what they found. Its blocks - * install the one `code-review` workflow; the reviewer roster itself is - * `@corbits/code-review`'s own `CODE_REVIEW_REVIEWERS` — mirrored into - * participants here rather than duplicated, so a reviewer's handle, - * name, and one-line role can never drift between the package that - * runs the review and the template that describes it. + * The code-review definition: three reviewer lenses over every pull + * request. Its blocks install the one `code-review` workflow; the + * reviewer roster itself is `@corbits/code-review`'s own + * `CODE_REVIEW_REVIEWERS` — mirrored into `agents` here rather than + * duplicated, so a reviewer's handle, name, and one-line role can never + * drift between the package that runs the review and the definition + * that describes it. */ -export const CODE_REVIEW_TEMPLATE: WorkbenchTemplateManifest = { +export const CODE_REVIEW_TEMPLATE: WorkbenchDefinition = { id: "code-review", title: "Code review", promise: @@ -254,8 +313,8 @@ export const CODE_REVIEW_TEMPLATE: WorkbenchTemplateManifest = { blocks: [{ assetName: "code-review", version: "0.0.1" }], // GitHub is the one thing this template cannot work without: no // repository, no diff to read and nowhere to post the review. - requiredConnections: ["github"], - optionalConnections: [], + plugins: { required: ["github"], optional: [] }, + tools: ["@corbits/github-tools"], routines: [], webhookTriggers: [ { @@ -266,13 +325,7 @@ export const CODE_REVIEW_TEMPLATE: WorkbenchTemplateManifest = { triggerFieldKey: "pullRequestUrl", }, ], - participants: [ - { - handle: "myra", - displayName: "Myra", - blockAssetName: "code-review", - role: "Talks through what the reviewers found and helps you decide what to act on.", - }, + agents: [ ...CODE_REVIEW_REVIEWERS.map((reviewer) => ({ handle: reviewer.handle, displayName: reviewer.displayName, @@ -290,6 +343,25 @@ export const CODE_REVIEW_TEMPLATE: WorkbenchTemplateManifest = { appliesToWebhookTrigger: "pull-request-opened", }, ], + onboardingSteps: [ + { + kind: "connect-plugin", + connectorId: "github", + title: "Connect GitHub", + why: "The reviewers need it to read your diffs and post what they'd change.", + }, + { + kind: "pick-github-repos", + title: "Pick your repos", + why: "Only the repositories you choose get reviewed.", + }, + { + kind: "start-webhook-trigger", + webhookTriggerKey: "pull-request-opened", + title: "Start reviewing", + why: "From now on, every new pull request in those repositories gets a review.", + }, + ], }; /** @@ -301,17 +373,17 @@ export const CODE_REVIEW_TEMPLATE: WorkbenchTemplateManifest = { * Exa (Scout's web-research tool) resolves through the keyless MCP * preset, so nothing here blocks the create on a connection. */ -export const DUE_DILIGENCE_TEMPLATE: WorkbenchTemplateManifest = { +export const DUE_DILIGENCE_TEMPLATE: WorkbenchDefinition = { id: "due-diligence", title: "Due Diligence", promise: "Scout checks a company, deal, or vendor against the web and what your team already knows, and saves what it finds so you can pick it up later.", blocks: [], - requiredConnections: [], - optionalConnections: ["exa"], + plugins: { required: [], optional: ["exa"] }, + tools: SCOUT_TOOL_PACKAGE_PINS.map((pin) => pin.name), routines: [], webhookTriggers: [], - participants: [ + agents: [ { handle: "myra", displayName: "Myra", @@ -324,9 +396,10 @@ export const DUE_DILIGENCE_TEMPLATE: WorkbenchTemplateManifest = { }, ], openInputs: [], + onboardingSteps: [], }; -export const WORKBENCH_TEMPLATES: readonly WorkbenchTemplateManifest[] = [ +export const WORKBENCH_TEMPLATES: readonly WorkbenchDefinition[] = [ GTM_TEMPLATE, CODE_REVIEW_TEMPLATE, DUE_DILIGENCE_TEMPLATE, @@ -336,25 +409,22 @@ const templateById = new Map( WORKBENCH_TEMPLATES.map((template) => [template.id, template]), ); -export function workbenchTemplate( - id: string, -): WorkbenchTemplateManifest | undefined { +export function workbenchTemplate(id: string): WorkbenchDefinition | undefined { return templateById.get(id); } /** - * Every asset name a template names — its blocks, and the block behind - * each routine and participant. A caller checking a template against + * Every asset name a definition names — its blocks, and the block behind + * each routine and agent. A caller checking a definition against * `WORKFLOW_CATALOG` walks this rather than three separate arrays. */ export function templateBlockAssetNames( - template: WorkbenchTemplateManifest, + definition: WorkbenchDefinition, ): readonly string[] { - return template.blocks.map((block) => block.assetName); + return definition.blocks.map((block) => block.assetName); } -/** The exact string a hub seeds into the bench library for one template. */ -/** The shipped templates as bench-library seed entries — the ONE +/** The shipped definitions as bench-library seed entries — the ONE * serialization every seeder uses (`apps/hub`'s boot seed and the eval * harness's scratch-hub seed), so the two can never drift. */ export function workbenchTemplateLibraryEntries(): readonly { @@ -363,107 +433,118 @@ export function workbenchTemplateLibraryEntries(): readonly { }[] { return WORKBENCH_TEMPLATES.map((template) => ({ id: template.id, - content: serializeWorkbenchTemplateManifest(template), + content: serializeWorkbenchDefinition(template), })); } -export function serializeWorkbenchTemplateManifest( - template: WorkbenchTemplateManifest, +export function serializeWorkbenchDefinition( + definition: WorkbenchDefinition, ): string { - return JSON.stringify(template, null, 2); + return JSON.stringify(definition, null, 2); } /** - * Parses a seeded library row's content back into a manifest, running + * Parses a seeded library row's content back into a definition, running * the same cross-reference checks module load runs on the shipped * constants. Throws on anything malformed — an unreadable library row * is a seeding defect to surface, never a shape to limp past. */ -export function parseWorkbenchTemplateManifest( - data: unknown, -): WorkbenchTemplateManifest { +export function parseWorkbenchDefinition(data: unknown): WorkbenchDefinition { const raw = typeof data === "string" ? JSON.parse(data) : data; - const parsed = WorkbenchTemplateManifestSchema(raw); + const parsed = WorkbenchDefinitionSchema(raw); if (parsed instanceof type.errors) { - throw new Error( - `workbench template manifest failed to parse: ${parsed.summary}`, - ); + throw new Error(`workbench definition failed to parse: ${parsed.summary}`); } assertValid(parsed); return parsed; } -function assertValid(template: WorkbenchTemplateManifest): void { - const blockNames = new Set(templateBlockAssetNames(template)); - const parsedBlocks = WorkbenchTemplateBlock.array()(template.blocks); +function assertValid(definition: WorkbenchDefinition): void { + const blockNames = new Set(templateBlockAssetNames(definition)); + const parsedBlocks = WorkbenchTemplateBlock.array()(definition.blocks); if (parsedBlocks instanceof type.errors) { throw new Error( - `workbench template "${template.id}" has an invalid blocks shape: ${parsedBlocks.summary}`, + `workbench definition "${definition.id}" has an invalid blocks shape: ${parsedBlocks.summary}`, ); } - const parsedRoutines = WorkbenchTemplateRoutine.array()(template.routines); + const parsedRoutines = WorkbenchTemplateRoutine.array()(definition.routines); if (parsedRoutines instanceof type.errors) { throw new Error( - `workbench template "${template.id}" has an invalid routines shape: ${parsedRoutines.summary}`, + `workbench definition "${definition.id}" has an invalid routines shape: ${parsedRoutines.summary}`, ); } - const parsedParticipants = WorkbenchTemplateParticipant.array()( - template.participants, - ); - if (parsedParticipants instanceof type.errors) { + const parsedAgents = WorkbenchDefinitionAgent.array()(definition.agents); + if (parsedAgents instanceof type.errors) { throw new Error( - `workbench template "${template.id}" has an invalid participants shape: ${parsedParticipants.summary}`, + `workbench definition "${definition.id}" has an invalid agents shape: ${parsedAgents.summary}`, ); } - const parsedInputs = WorkbenchTemplateOpenInput.array()(template.openInputs); + const parsedInputs = WorkbenchTemplateOpenInput.array()( + definition.openInputs, + ); if (parsedInputs instanceof type.errors) { throw new Error( - `workbench template "${template.id}" has an invalid openInputs shape: ${parsedInputs.summary}`, + `workbench definition "${definition.id}" has an invalid openInputs shape: ${parsedInputs.summary}`, ); } const parsedWebhookTriggers = WorkbenchTemplateWebhookTrigger.array()( - template.webhookTriggers, + definition.webhookTriggers, ); if (parsedWebhookTriggers instanceof type.errors) { throw new Error( - `workbench template "${template.id}" has an invalid webhookTriggers shape: ${parsedWebhookTriggers.summary}`, + `workbench definition "${definition.id}" has an invalid webhookTriggers shape: ${parsedWebhookTriggers.summary}`, + ); + } + const parsedSteps = WorkbenchOnboardingStep.array()( + definition.onboardingSteps, + ); + if (parsedSteps instanceof type.errors) { + throw new Error( + `workbench definition "${definition.id}" has an invalid onboardingSteps shape: ${parsedSteps.summary}`, ); } - const routineKeys = new Set(template.routines.map((routine) => routine.key)); - for (const routine of template.routines) { + if (new Set(definition.tools).size !== definition.tools.length) { + throw new Error( + `workbench definition "${definition.id}" names the same tool package more than once`, + ); + } + const routineKeys = new Set( + definition.routines.map((routine) => routine.key), + ); + for (const routine of definition.routines) { if (!blockNames.has(routine.blockAssetName)) { throw new Error( - `workbench template "${template.id}" routine "${routine.key}" runs "${routine.blockAssetName}", which the template does not install`, + `workbench definition "${definition.id}" routine "${routine.key}" runs "${routine.blockAssetName}", which the definition does not install`, ); } } const webhookTriggerKeys = new Set( - template.webhookTriggers.map((trigger) => trigger.key), + definition.webhookTriggers.map((trigger) => trigger.key), ); - for (const trigger of template.webhookTriggers) { + for (const trigger of definition.webhookTriggers) { if (!blockNames.has(trigger.blockAssetName)) { throw new Error( - `workbench template "${template.id}" webhook trigger "${trigger.key}" fires "${trigger.blockAssetName}", which the template does not install`, + `workbench definition "${definition.id}" webhook trigger "${trigger.key}" fires "${trigger.blockAssetName}", which the definition does not install`, ); } } - for (const participant of template.participants) { + for (const agent of definition.agents) { if ( - participant.blockAssetName !== undefined && - !blockNames.has(participant.blockAssetName) + agent.blockAssetName !== undefined && + !blockNames.has(agent.blockAssetName) ) { throw new Error( - `workbench template "${template.id}" participant "${participant.handle}" is backed by "${participant.blockAssetName}", which the template does not install`, + `workbench definition "${definition.id}" agent "${agent.handle}" is backed by "${agent.blockAssetName}", which the definition does not install`, ); } } - for (const input of template.openInputs) { + for (const input of definition.openInputs) { const appliesToCount = Number(input.appliesToRoutine !== undefined) + Number(input.appliesToWebhookTrigger !== undefined); if (appliesToCount !== 1) { throw new Error( - `workbench template "${template.id}" input "${input.key}" must apply to exactly one of a routine or a webhook trigger`, + `workbench definition "${definition.id}" input "${input.key}" must apply to exactly one of a routine or a webhook trigger`, ); } if ( @@ -471,7 +552,7 @@ function assertValid(template: WorkbenchTemplateManifest): void { !routineKeys.has(input.appliesToRoutine) ) { throw new Error( - `workbench template "${template.id}" input "${input.key}" applies to routine "${input.appliesToRoutine}", which the template does not create`, + `workbench definition "${definition.id}" input "${input.key}" applies to routine "${input.appliesToRoutine}", which the definition does not create`, ); } if ( @@ -479,12 +560,88 @@ function assertValid(template: WorkbenchTemplateManifest): void { !webhookTriggerKeys.has(input.appliesToWebhookTrigger) ) { throw new Error( - `workbench template "${template.id}" input "${input.key}" applies to webhook trigger "${input.appliesToWebhookTrigger}", which the template does not create`, + `workbench definition "${definition.id}" input "${input.key}" applies to webhook trigger "${input.appliesToWebhookTrigger}", which the definition does not create`, ); } } + assertOnboardingWalkthrough(definition); +} + +/** + * The walkthrough is ordered, so its checks are ordered too: a step can + * only ask for something an earlier step made possible, and a required + * plugin with no step to connect it is a definition that promises setup + * it never asks for. + */ +function assertOnboardingWalkthrough(definition: WorkbenchDefinition): void { + const declaredPlugins = new Set([ + ...definition.plugins.required, + ...definition.plugins.optional, + ]); + const connectedSoFar = new Set(); + const startedTriggerKeys = new Set(); + let pickedRepos = false; + + for (const step of definition.onboardingSteps) { + if (step.kind === "connect-plugin") { + if (!declaredPlugins.has(step.connectorId)) { + throw new Error( + `workbench definition "${definition.id}" onboarding connects "${step.connectorId}", which is not one of its plugins`, + ); + } + connectedSoFar.add(step.connectorId); + continue; + } + if (step.kind === "pick-github-repos") { + if (!connectedSoFar.has("github")) { + throw new Error( + `workbench definition "${definition.id}" asks for repos before its onboarding connects "github"`, + ); + } + pickedRepos = true; + continue; + } + if (!webhookTriggerKeyExists(definition, step.webhookTriggerKey)) { + throw new Error( + `workbench definition "${definition.id}" onboarding starts webhook trigger "${step.webhookTriggerKey}", which the definition does not create`, + ); + } + if (startedTriggerKeys.has(step.webhookTriggerKey)) { + throw new Error( + `workbench definition "${definition.id}" onboarding starts webhook trigger "${step.webhookTriggerKey}" more than once`, + ); + } + if (!pickedRepos) { + throw new Error( + `workbench definition "${definition.id}" starts webhook trigger "${step.webhookTriggerKey}" before its onboarding picks repos`, + ); + } + startedTriggerKeys.add(step.webhookTriggerKey); + } + + for (const connectorId of definition.plugins.required) { + if (!connectedSoFar.has(connectorId)) { + throw new Error( + `workbench definition "${definition.id}" requires plugin "${connectorId}" but its onboarding never asks anyone to connect it`, + ); + } + } + for (const trigger of definition.webhookTriggers) { + if (!startedTriggerKeys.has(trigger.key)) { + throw new Error( + `workbench definition "${definition.id}" webhook trigger "${trigger.key}" is never started by an onboarding step`, + ); + } + } +} + +function webhookTriggerKeyExists( + definition: WorkbenchDefinition, + key: string, +): boolean { + return definition.webhookTriggers.some((trigger) => trigger.key === key); } -for (const template of WORKBENCH_TEMPLATES) { - assertValid(template); +for (const definition of WORKBENCH_TEMPLATES) { + assertValid(definition); } diff --git a/packages/workflow-catalog/test/instantiate.test.ts b/packages/workflow-catalog/test/instantiate.test.ts index f608f66f7..cc11ccac5 100644 --- a/packages/workflow-catalog/test/instantiate.test.ts +++ b/packages/workflow-catalog/test/instantiate.test.ts @@ -5,6 +5,7 @@ import { CODE_REVIEW_TEMPLATE, DUE_DILIGENCE_TEMPLATE, GTM_TEMPLATE, + type WorkbenchOnboardingStep, } from "../src/index"; import { instantiateWorkbenchTemplate, @@ -19,16 +20,22 @@ function fakePorts( readonly invited: string[]; readonly recordedConnections: (readonly string[])[]; readonly deployedBlocks: string[]; + readonly calls: string[]; + readonly onboardingSteps: (readonly WorkbenchOnboardingStep[])[]; } { const created: string[] = []; const invited: string[] = []; const recordedConnections: (readonly string[])[] = []; const deployedBlocks: string[] = []; + const calls: string[] = []; + const onboardingSteps: (readonly WorkbenchOnboardingStep[])[] = []; return { created, invited, recordedConnections, deployedBlocks, + calls, + onboardingSteps, async listAgentHandles() { return existingHandles.map((handle) => ({ handle, id: `def-${handle}` })); }, @@ -42,14 +49,20 @@ function fakePorts( }, async inviteParticipantAgent(id) { invited.push(id); + calls.push(`invite:${id}`); }, async recordPendingConnections(pendingConnections) { recordedConnections.push(pendingConnections); + calls.push("recordPendingConnections"); + }, + async beginOnboarding(steps) { + onboardingSteps.push(steps); + calls.push("beginOnboarding"); }, }; } -test("instantiating the code-review template creates the three reviewer definitions, never Myra", () => { +test("instantiating the code-review definition creates the three reviewer definitions, never Myra", () => { const ports = fakePorts(); return instantiateWorkbenchTemplate(CODE_REVIEW_TEMPLATE, ports).then( (result) => { @@ -93,7 +106,7 @@ test("instantiating the code-review template skips a reviewer that already exist // 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 () => { +test("instantiating the code-review definition invites every reviewer into the room, created or skipped alike", async () => { const ports = fakePorts(["architecture-reviewer"]); const result = await instantiateWorkbenchTemplate( CODE_REVIEW_TEMPLATE, @@ -112,23 +125,53 @@ test("instantiating the code-review template invites every reviewer into the roo expect(ports.invited).toHaveLength(3); }); -test("instantiating the code-review template invites existing Myra, never creates her", async () => { +test("the code-review definition never invites or creates Myra — nobody hosts a template room", async () => { const ports = fakePorts(["assistant"]); - await instantiateWorkbenchTemplate(CODE_REVIEW_TEMPLATE, ports); - expect(ports.invited).toContain("def-assistant"); + const result = await instantiateWorkbenchTemplate( + CODE_REVIEW_TEMPLATE, + ports, + ); + expect(ports.invited).not.toContain("def-assistant"); expect(ports.created).not.toContain("myra"); expect(ports.created).not.toContain("assistant"); + expect(result.invitedHandles).not.toContain("myra"); +}); + +test("the due-diligence definition still invites an existing assistant as Myra", async () => { + const ports = fakePorts(["assistant"]); + const result = await instantiateWorkbenchTemplate( + DUE_DILIGENCE_TEMPLATE, + ports, + ); + expect(ports.invited).toContain("def-assistant"); + expect(ports.created).not.toContain("myra"); + expect(result.invitedHandles).toEqual(["myra", "scout"]); }); -test("instantiating the code-review template names an honest pending note for its not-yet-scoped webhook trigger", async () => { +test("instantiating the code-review definition begins its walkthrough, in definition order, once everything else is in place", async () => { const ports = fakePorts(); const result = await instantiateWorkbenchTemplate( CODE_REVIEW_TEMPLATE, ports, ); - expect(result.webhookTriggerTodos).toHaveLength(1); - expect(result.webhookTriggerTodos[0]).toContain("pull-request-opened"); - expect(result.webhookTriggerTodos[0]).toContain("connect-github-setup"); + expect(ports.onboardingSteps).toEqual([CODE_REVIEW_TEMPLATE.onboardingSteps]); + expect(result.onboardingSteps).toEqual(CODE_REVIEW_TEMPLATE.onboardingSteps); + expect(ports.calls.at(-1)).toBe("beginOnboarding"); + expect(ports.calls.at(-2)).toBe("recordPendingConnections"); + expect(ports.calls.filter((call) => call.startsWith("invite:"))).toHaveLength( + 3, + ); +}); + +test("a definition with no onboarding steps never begins a walkthrough", async () => { + const ports = fakePorts(); + const result = await instantiateWorkbenchTemplate( + DUE_DILIGENCE_TEMPLATE, + ports, + ); + expect(ports.onboardingSteps).toEqual([]); + expect(ports.calls).not.toContain("beginOnboarding"); + expect(result.onboardingSteps).toEqual([]); }); test("instantiating the code-review template deploys its referenced code-review block workflow", async () => { @@ -152,7 +195,7 @@ test("a block workflow the tenant already deployed is reported as skipped, never expect(result.skippedBlockAssetNames).toEqual(["code-review"]); }); -test("instantiating a manifest with a participant outside the reviewer roster throws rather than silently skipping it", () => { +test("instantiating a definition with an agent outside the reviewer roster throws rather than silently skipping it", () => { return expect( instantiateWorkbenchTemplate(GTM_TEMPLATE, fakePorts()), ).rejects.toThrow(/has no known create-agent request/); @@ -208,6 +251,9 @@ test("Scout's create request carries its tool package pins", async () => { async recordPendingConnections() { /* noop */ }, + async beginOnboarding() { + /* noop */ + }, }; await instantiateWorkbenchTemplate(DUE_DILIGENCE_TEMPLATE, ports); const scout = requests.find((request) => request.handle === "scout"); @@ -220,20 +266,23 @@ test("Scout's create request carries its tool package pins", async () => { ); }); -// CL-6499 dropped Jimmy's own template (he is not a "kind of workbench"); +// No shipped definition names Jimmy (he is not a "kind of workbench"); // `@corbits/chat-ui`'s "Add Jimmy" quick-create row calls -// `jimmyAgentRequest()` directly instead of going through a manifest. This +// `jimmyAgentRequest()` directly instead of going through one. This // proves `instantiateWorkbenchTemplate`'s request map still resolves his // handle, so a future template naming him works with no new plumbing. -test("a manifest naming Jimmy's handle still resolves and creates him", async () => { - const manifestNamingJimmy = { +test("a definition naming Jimmy's handle still resolves and creates him", async () => { + const definitionNamingJimmy = { ...DUE_DILIGENCE_TEMPLATE, - participants: [ + agents: [ { handle: "jimmy", displayName: "Jimmy", role: "Replies with a GIF." }, ], }; const ports = fakePorts(); - const result = await instantiateWorkbenchTemplate(manifestNamingJimmy, ports); + const result = await instantiateWorkbenchTemplate( + definitionNamingJimmy, + ports, + ); expect(result.createdHandles).toEqual(["jimmy"]); expect(ports.created).toEqual(["jimmy"]); }); diff --git a/packages/workflow-catalog/test/templates.test.ts b/packages/workflow-catalog/test/templates.test.ts index d89f68144..21a941a32 100644 --- a/packages/workflow-catalog/test/templates.test.ts +++ b/packages/workflow-catalog/test/templates.test.ts @@ -1,6 +1,7 @@ import { expect, test } from "bun:test"; import { CONNECTOR_REGISTRY } from "@workbench/connections/registry"; import { MCP_PRESETS } from "@workbench/connections/mcp-presets"; +import { SCOUT_TOOL_PACKAGE_PINS } from "@corbits/scout-agent/definition"; import { CODE_REVIEW_TEMPLATE, @@ -10,8 +11,8 @@ import { WORKFLOW_CATALOG, templateBlockAssetNames, workbenchTemplate, - parseWorkbenchTemplateManifest, - serializeWorkbenchTemplateManifest, + parseWorkbenchDefinition, + serializeWorkbenchDefinition, workflowCatalogEntry, } from "../src/index"; @@ -31,8 +32,8 @@ test("every connector a template names is a real connector or MCP preset", () => ]); for (const template of WORKBENCH_TEMPLATES) { for (const connector of [ - ...template.requiredConnections, - ...template.optionalConnections, + ...template.plugins.required, + ...template.plugins.optional, ]) { expect(known).toContain(connector); } @@ -41,8 +42,8 @@ test("every connector a template names is a real connector or MCP preset", () => test("a template never both requires and merely offers the same connector", () => { for (const template of WORKBENCH_TEMPLATES) { - const required = new Set(template.requiredConnections); - for (const optional of template.optionalConnections) { + const required = new Set(template.plugins.required); + for (const optional of template.plugins.optional) { expect(required).not.toContain(optional); } } @@ -70,8 +71,8 @@ test("the GTM template installs the four ported v1 workflows plus the call write }); test("the GTM template blocks the create on the two connectors nothing works without", () => { - expect(GTM_TEMPLATE.requiredConnections).toEqual(["attio", "exa"]); - expect(GTM_TEMPLATE.optionalConnections).toEqual(["granola"]); + expect(GTM_TEMPLATE.plugins.required).toEqual(["attio", "exa"]); + expect(GTM_TEMPLATE.plugins.optional).toEqual(["granola"]); }); test("the GTM template schedules call discovery and the web watch, and nothing else", () => { @@ -136,7 +137,7 @@ test("the code-review template installs the code-review workflow and requires gi expect(templateBlockAssetNames(CODE_REVIEW_TEMPLATE)).toEqual([ "code-review", ]); - expect(CODE_REVIEW_TEMPLATE.requiredConnections).toEqual(["github"]); + expect(CODE_REVIEW_TEMPLATE.plugins.required).toEqual(["github"]); }); test("the code-review template fires from a pull-request webhook, not a clock", () => { @@ -148,66 +149,85 @@ test("the code-review template fires from a pull-request webhook, not a clock", expect(trigger?.triggerFieldKey).toBe("pullRequestUrl"); }); -test("the code-review template's participants are Myra and the three reviewers", () => { - expect( - CODE_REVIEW_TEMPLATE.participants.map((participant) => participant.handle), - ).toEqual([ - "myra", +test("the code-review definition's agents are the three reviewers and nobody else", () => { + expect(CODE_REVIEW_TEMPLATE.agents.map((agent) => agent.handle)).toEqual([ "correctness-reviewer", "architecture-reviewer", "release-risk-reviewer", ]); }); -test("participants are addressable by a distinct handle", () => { +test("the code-review definition names the GitHub tool package its reviewers need", () => { + expect(CODE_REVIEW_TEMPLATE.tools).toEqual(["@corbits/github-tools"]); +}); + +test("the code-review walkthrough is connect GitHub, pick repos, start reviewing — in that order", () => { + expect(CODE_REVIEW_TEMPLATE.onboardingSteps.map((step) => step.kind)).toEqual( + ["connect-plugin", "pick-github-repos", "start-webhook-trigger"], + ); + for (const step of CODE_REVIEW_TEMPLATE.onboardingSteps) { + expect(step.title.length).toBeGreaterThan(0); + expect(step.why.length).toBeGreaterThan(0); + } +}); + +test("agents are addressable by a distinct handle", () => { for (const template of WORKBENCH_TEMPLATES) { - const handles = template.participants.map( - (participant) => participant.handle, - ); + const handles = template.agents.map((agent) => agent.handle); expect(new Set(handles).size).toBe(handles.length); } }); -test("the due-diligence template's participants are Myra and Scout, neither backed by a block", () => { +test("a definition never names the same tool package twice", () => { + for (const template of WORKBENCH_TEMPLATES) { + expect(new Set(template.tools).size).toBe(template.tools.length); + } +}); + +test("the due-diligence definition's agents are Myra and Scout, neither backed by a block", () => { expect(workbenchTemplate("due-diligence")).toBe(DUE_DILIGENCE_TEMPLATE); - expect( - DUE_DILIGENCE_TEMPLATE.participants.map( - (participant) => participant.handle, - ), - ).toEqual(["myra", "scout"]); + expect(DUE_DILIGENCE_TEMPLATE.agents.map((agent) => agent.handle)).toEqual([ + "myra", + "scout", + ]); expect(templateBlockAssetNames(DUE_DILIGENCE_TEMPLATE)).toEqual([]); - for (const participant of DUE_DILIGENCE_TEMPLATE.participants) { - expect(participant.blockAssetName).toBeUndefined(); + for (const agent of DUE_DILIGENCE_TEMPLATE.agents) { + expect(agent.blockAssetName).toBeUndefined(); } }); +test("the due-diligence definition names Scout's tool packages and needs no walkthrough", () => { + expect(DUE_DILIGENCE_TEMPLATE.tools).toEqual( + SCOUT_TOOL_PACKAGE_PINS.map((pin) => pin.name), + ); + expect(DUE_DILIGENCE_TEMPLATE.onboardingSteps).toEqual([]); +}); + test("the due-diligence template blocks the create on nothing — Exa is offered, never required", () => { - expect(DUE_DILIGENCE_TEMPLATE.requiredConnections).toEqual([]); - expect(DUE_DILIGENCE_TEMPLATE.optionalConnections).toEqual(["exa"]); + expect(DUE_DILIGENCE_TEMPLATE.plugins.required).toEqual([]); + expect(DUE_DILIGENCE_TEMPLATE.plugins.optional).toEqual(["exa"]); }); test("Jimmy is not a workbench template — the picker offers no such kind of workbench", () => { expect(workbenchTemplate("default-teammates")).toBeUndefined(); for (const template of WORKBENCH_TEMPLATES) { - expect( - template.participants.some( - (participant) => participant.handle === "jimmy", - ), - ).toBe(false); + expect(template.agents.some((agent) => agent.handle === "jimmy")).toBe( + false, + ); } }); -test("every shipped template survives the seed round trip verbatim", () => { +test("every shipped definition survives the seed round trip verbatim", () => { for (const template of WORKBENCH_TEMPLATES) { - const parsed = parseWorkbenchTemplateManifest( - serializeWorkbenchTemplateManifest(template), + const parsed = parseWorkbenchDefinition( + serializeWorkbenchDefinition(template), ); expect(parsed).toEqual(template); } }); test("a malformed library row fails to parse instead of half-loading", () => { - expect(() => parseWorkbenchTemplateManifest('{"id":"code-review"}')).toThrow( + expect(() => parseWorkbenchDefinition('{"id":"code-review"}')).toThrow( /failed to parse/, ); const firstRoutine = GTM_TEMPLATE.routines[0]; @@ -217,8 +237,70 @@ test("a malformed library row fails to parse instead of half-loading", () => { routines: [{ ...firstRoutine, blockAssetName: "not-installed" }], }; expect(() => - parseWorkbenchTemplateManifest( - serializeWorkbenchTemplateManifest(orphanRoutine), - ), + parseWorkbenchDefinition(serializeWorkbenchDefinition(orphanRoutine)), ).toThrow(/does not install/); }); + +test("a required plugin with no step to connect it fails loud, naming the plugin", () => { + const unaskedPlugin = { + ...DUE_DILIGENCE_TEMPLATE, + plugins: { required: ["exa"], optional: [] }, + }; + expect(() => + parseWorkbenchDefinition(serializeWorkbenchDefinition(unaskedPlugin)), + ).toThrow(/requires plugin "exa" but its onboarding never asks/); +}); + +test("a walkthrough step naming a connector outside the definition's plugins fails loud", () => { + const strayConnector = { + ...DUE_DILIGENCE_TEMPLATE, + onboardingSteps: [ + { + kind: "connect-plugin" as const, + connectorId: "attio", + title: "Connect Attio", + why: "Nothing here reads Attio.", + }, + ], + }; + expect(() => + parseWorkbenchDefinition(serializeWorkbenchDefinition(strayConnector)), + ).toThrow(/connects "attio", which is not one of its plugins/); +}); + +test("a start-webhook-trigger step naming an unknown trigger fails loud", () => { + const unknownTrigger = { + ...CODE_REVIEW_TEMPLATE, + onboardingSteps: [ + ...CODE_REVIEW_TEMPLATE.onboardingSteps.slice(0, 2), + { + kind: "start-webhook-trigger" as const, + webhookTriggerKey: "issue-opened", + title: "Start reviewing", + why: "Nothing fires on issues.", + }, + ], + }; + expect(() => + parseWorkbenchDefinition(serializeWorkbenchDefinition(unknownTrigger)), + ).toThrow(/"issue-opened", which the definition does not create/); +}); + +test("picking repos before GitHub is connected fails loud — the walkthrough is ordered", () => { + const outOfOrder = { + ...CODE_REVIEW_TEMPLATE, + onboardingSteps: [ + { + kind: "pick-github-repos" as const, + title: "Pick your repos", + why: "Only the repositories you choose get reviewed.", + }, + ...CODE_REVIEW_TEMPLATE.onboardingSteps.filter( + (step) => step.kind !== "pick-github-repos", + ), + ], + }; + expect(() => + parseWorkbenchDefinition(serializeWorkbenchDefinition(outOfOrder)), + ).toThrow(/asks for repos before its onboarding connects "github"/); +}); From 56f68ca4b9b9c43c2cdd47d33fdb9036878c0ab7 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 27 Aug 2026 15:48:32 -0700 Subject: [PATCH 3/3] Update docs: Workbench Definition owns template create --- ARCHITECTURE.md | 22 ++++++++++++++++++++ IMPLEMENTATION.md | 33 ++++++++++++++++++++++++++---- PRODUCT.md | 45 +++++++++++++++++++++-------------------- docs/GLOSSARY.md | 47 ++++++++++++++++++++++--------------------- docs/connect-cards.md | 26 ++++++++++++++++-------- 5 files changed, 116 insertions(+), 57 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index c6a3ca4ef..518e41b4f 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -110,6 +110,26 @@ one path, deltas to pixels: synthetic in-progress message alongside the persisted timeline, until the real message lands and replaces it. +## Workbench Definition and hostless onboarding + +A picker "template" is a shipped **Workbench Definition**, not a second +kind of object: default agents, routines, tools, required and optional +plugins, and an ordered onboarding walkthrough. Creating from a named +row mints an empty workbench channel with no host, then instantiates +that definition into the room. The walkthrough is posted as a system +timeline card, never as a side effect of hosting an agent — so an +empty channel can onboard with nobody launched. + +Code review's definition names three reviewers and does not name Myra. +GitHub already connected is the same in-room card: it reads live +credential state and flips to repository pick. There is no separate +create-dialog path for the already-connected case. + +Settling a connector a template room is waiting on records the +connected event from a system address and does not wake an agent. A +generic in-room connect that an agent asked for still wakes that +agent. + ## Capability growth and approval gates An agent's capability set grows through what it is granted, not through @@ -145,6 +165,8 @@ does not maintain a parallel scheduler. - [docs/workbench-tenancy.md](docs/workbench-tenancy.md) — workbench tenant mint, listing, and move mechanics - [docs/needs-you.md](docs/needs-you.md) — the approval surfacing model +- [docs/connect-cards.md](docs/connect-cards.md) — in-room connect cards + and template-room settle - [VENDORED.md](VENDORED.md) — the vendoring ledger for `@intx/*` ## Open questions diff --git a/IMPLEMENTATION.md b/IMPLEMENTATION.md index 1e5f5eaef..e485dee54 100644 --- a/IMPLEMENTATION.md +++ b/IMPLEMENTATION.md @@ -103,16 +103,41 @@ Deployment is explicit via **Pulumi**, targeting **Railway**. CI runs tests only — nothing auto-deploys on `main`; a deploy is a deliberate, separate action. +## Workbench Definition (shipped) + +`WorkbenchDefinition` (`WorkbenchDefinitionSchema` in +`@corbits/workflow-catalog`) is the one type for a named picker row: +default agents, routines, tools, plugins `{required, optional}`, and +ordered `onboardingSteps`. A template is a shipped definition, not a +second kind. `instantiateWorkbenchTemplate` resolves the definition +against a bench over injected ports, including `beginOnboarding(steps)`. + +Create (`apps/web`'s `instant-agent-create.ts`) mints an empty +`kind: "workbench"` channel with no host and no `definitionId`, +instantiates, then `POST /workbenches/:id/onboarding` +(`packages/chat/src/routes.ts`) posts the walkthrough from +`system@` — a `connect-github` block carrying the +definition's title, promise, and step labels. The card is not posted as +a side effect of hosting an agent. + +`settleConnectedService` (`packages/chat/src/connect-pending.ts`) +clears both `connections/pending` and `template/pendingConnections`. A +template-key-only match posts `connection.connected` from the system +address and does not `dispatchTurn`. A generic pending match still +wakes the asking agent. + ## GitHub connect (shipped) The in-room `connect-github` card and the Plugins/Connections GitHub row are **PAT-first** (CL-6345): the person pastes a personal access token; the host tests and stores it through `@workbench/connections`' generic `github/complete` route. The card then flips in place to pick repos -(`startReviewingRepos`). A GitHub App / hosted OAuth welcome mat is -CL-6343 and is not the current product path — do not document OAuth as -the first Connect step, and do not treat a PAT paste as a defect against -an OAuth-first welcome mat that has not shipped. +(`startReviewingRepos`), including when GitHub is already connected — +the in-room card reads live state; there is no `/new` already-connected +dialog. A GitHub App / hosted OAuth welcome mat is CL-6343 and is not +the current product path — do not document OAuth as the first Connect +step, and do not treat a PAT paste as a defect against an OAuth-first +welcome mat that has not shipped. Optional `GITHUB_APP_CLIENT_ID` / `GITHUB_APP_CLIENT_SECRET` exist for that future hosted path; leaving them unset is normal. See diff --git a/PRODUCT.md b/PRODUCT.md index 7b9ad1ffb..3f27815a0 100644 --- a/PRODUCT.md +++ b/PRODUCT.md @@ -70,9 +70,10 @@ Create stays on `/new` (`apps/web/src/pages/new-workbench-picker.tsx`): a prompt box is the primary act: typing a goal and submitting mints an empty channel and sends that text as the first message; blank plus invites nobody. Named-template rows underneath mint that same empty -channel, then invite existing principals (including Myra as a -participant, never as mint `definitionId`) — one-click shortcuts, not -a kind-then-Create second step. There is no Describe door and no +channel with no host and no mint `definitionId`, then instantiate the +picked Workbench Definition's own agents (Myra joins only when the +definition names her) — one-click shortcuts, not a kind-then-Create +second step. There is no Describe door and no `describe-first-workbench.tsx`. A bench that already has one or more workbenches skips first-run and @@ -88,19 +89,19 @@ put there. ### Code review's first minute -Code review is the product scene for the template path: Connect GitHub -with a personal access token (the shipped path today) → the connect card -flips in place to pick repositories → reviewers introduce themselves as -left-aligned messages with avatars. A GitHub App / hosted OAuth welcome -mat is future work (CL-6343), not current product. +Code review is the product scene for the definition-driven path: +minting the Code review workbench opens an empty room with no host — +its Workbench Definition names three reviewers and no Myra. The room +itself posts the onboarding card: Connect GitHub with a personal access +token (the shipped path today). The same in-room card reads live +connection state and flips in place to pick repositories — already +connected GitHub is that card, not a `/new` dialog. A GitHub App / +hosted OAuth welcome mat is future work (CL-6343), not current product. -Connected/settle honesty — no stale Connect after success; settle never -posting as the signed-in user; no agent 401 after GitHub already -succeeded — is the **target**, not shipped end-to-end. What ships today -is narrower: live credential reads that resolve at call, and a GitHub -settle path that can still attribute as the connecting user. Point -implementers at IMPLEMENTATION.md open questions, CL-6737, and CL-6738; -do not treat those three guarantees as current product law. +Settle for a template-key-only wait posts from a system sender and +does not wake an agent. Generic `connections/pending` still wakes the +asking agent. Neither path posts the connected notice as the connecting +person. ## Plugins and Skills @@ -171,8 +172,9 @@ User-facing surfaces (UI, docs, support) use exactly these nouns: - **DM** — the one 1:1 conversation with an agent. Never cloned by a second open. - **Channel** — a shared room between people and agents. Plus mints an - empty one; nobody is auto-hosted. Named templates invite existing - principals into that room. + empty one; nobody is auto-hosted. Named templates instantiate their + Workbench Definition's own agents into that room (Myra joins only + when the definition names her). - **Bench** — the shared team scope a person signs into and switches between; shown in the bench switcher, never called a "workspace" or "org" in copy. @@ -186,11 +188,10 @@ user-facing surfaces use the rest of the product vocabulary above. ## Open questions -- Connected/settle honesty (no stale Connect after success; settle never - posting as the signed-in user; no agent 401 after GitHub already - succeeded) stays **target** until CL-6737 and CL-6738 land — see - IMPLEMENTATION.md open questions; do not document those guarantees as - shipped. +- Template-key-only settle (system sender, no agent wake) is shipped; + generic `connections/pending` still wakes the asking agent. A leftover + agent 401 after GitHub already succeeded is still a first-minute bug + — see IMPLEMENTATION.md; do not document that it cannot happen. - The precise boundary of what Insights surfaces to a non-admin bench member (all tenant activity vs. only their own) is not spelled out in `packages/insights`'s own docs as of this writing. diff --git a/docs/GLOSSARY.md b/docs/GLOSSARY.md index a9b780e5e..40aab28dc 100644 --- a/docs/GLOSSARY.md +++ b/docs/GLOSSARY.md @@ -9,29 +9,30 @@ Interchange's, so its code identifiers, wire fields, and route segments were cut over to match the product word directly, with no separate lower-level name left to list. -| Product term | Platform term | What it is | -| ---------------- | ------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| **Bench** | tenant | A shared space where a team and its agents work — members, definitions, runs, and grants live here | -| **User** | principal | An identity that can act in a bench — human or agent | -| **Agent** | principal (agent) | A named coworker principal, not a template; opening the row reopens that agent's one DM. The sidebar mixes that DM with channels in one recency list (pins first) | -| **DM** | kind: chat | The one 1:1 tenant with that agent — two opens never clone a second DM | -| **Channel** | kind: workbench | A shared room between people and agents (multi-principal tenant). `+` mints an empty one with nobody hosted; named templates mint the same empty channel, then invite existing principals | -| **Definition** | workflow definition | A deployable unit of agent behavior, authored as code — not a template you mint into many conversations | -| **Run** | workflow run | A definition executing in a bench; interactive runs carry conversations | -| **Routine** | — | The named parent entity over runs of one definition — a trigger (or none), a delivery workbench, and its run history; see [`@corbits/routines`](../packages/routines/README.md) | -| **Approval** | approval | A human decision gating an external side effect | -| **Grant** | grant | Permission for a principal to act on a resource | -| **Hub** | hub | The API and coordination service a bench lives on | -| **Sidecar** | sidecar | The execution host that runs definitions on behalf of a hub | -| **Extension** | — | A route factory mounted on the hub to add product surface | -| **Workbenches** | — | The product name and the mint verb ("New workbench"), not a sidebar heading. Conversation tenants (agent DMs and channels) share one recency list, pins first — not two labeled sections | -| **Workbench** | — | The one conversation surface: an agent conversation (named by its agent) or a multi-party conversation (named by its own title) — durable hub-side data with no run of its own; also its own tenant, parented under the bench it was created in, so its membership and grants are its own — see [CHAT.md](CHAT.md) and [workbench-tenancy.md](workbench-tenancy.md) | -| **Timeline** | — | A workbench's own message rows, read back in order, as the conversation record | -| **Participant** | — | An address (human or agent) a workbench's settings list as able to post or be mentioned | -| **Handle** | — | A participant's short, unique-within-workbench mention name (e.g. `echo`), distinct from its address | -| **Mention** | — | `@` plus a participant's handle in message text, triggering fan-out to that participant | -| **Reply bridge** | — | The bridge that turns an invited agent's `connector.reply` events into workbench timeline messages | -| **Concept** | — | A kind of work an agent asks for a model by — `cheap-loop`, `code-work`, `image-reader` — resolved against the bench into an ordered, priced chain of the models it can actually reach; agents never name a model, see [inference-concepts.md](inference-concepts.md) | +| Product term | Platform term | What it is | +| ------------------------ | ------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Bench** | tenant | A shared space where a team and its agents work — members, definitions, runs, and grants live here | +| **User** | principal | An identity that can act in a bench — human or agent | +| **Agent** | principal (agent) | A named coworker principal, not a template; opening the row reopens that agent's one DM. The sidebar mixes that DM with channels in one recency list (pins first) | +| **DM** | kind: chat | The one 1:1 tenant with that agent — two opens never clone a second DM | +| **Channel** | kind: workbench | A shared room between people and agents (multi-principal tenant). `+` mints an empty one with nobody hosted; named templates mint the same empty channel, then instantiate their Workbench Definition and run its onboarding walkthrough in the room | +| **Definition** | workflow definition | A deployable unit of agent behavior, authored as code — not a template you mint into many conversations | +| **Workbench Definition** | — | The arktype description a named template instantiates: its default agents, routines, tools, required/optional plugins, and its ordered onboarding walkthrough — see `packages/workflow-catalog/src/templates.ts`'s `WorkbenchDefinitionSchema` | +| **Run** | workflow run | A definition executing in a bench; interactive runs carry conversations | +| **Routine** | — | The named parent entity over runs of one definition — a trigger (or none), a delivery workbench, and its run history; see [`@corbits/routines`](../packages/routines/README.md) | +| **Approval** | approval | A human decision gating an external side effect | +| **Grant** | grant | Permission for a principal to act on a resource | +| **Hub** | hub | The API and coordination service a bench lives on | +| **Sidecar** | sidecar | The execution host that runs definitions on behalf of a hub | +| **Extension** | — | A route factory mounted on the hub to add product surface | +| **Workbenches** | — | The product name and the mint verb ("New workbench"), not a sidebar heading. Conversation tenants (agent DMs and channels) share one recency list, pins first — not two labeled sections | +| **Workbench** | — | The one conversation surface: an agent conversation (named by its agent) or a multi-party conversation (named by its own title) — durable hub-side data with no run of its own; also its own tenant, parented under the bench it was created in, so its membership and grants are its own — see [CHAT.md](CHAT.md) and [workbench-tenancy.md](workbench-tenancy.md) | +| **Timeline** | — | A workbench's own message rows, read back in order, as the conversation record | +| **Participant** | — | An address (human or agent) a workbench's settings list as able to post or be mentioned | +| **Handle** | — | A participant's short, unique-within-workbench mention name (e.g. `echo`), distinct from its address | +| **Mention** | — | `@` plus a participant's handle in message text, triggering fan-out to that participant | +| **Reply bridge** | — | The bridge that turns an invited agent's `connector.reply` events into workbench timeline messages | +| **Concept** | — | A kind of work an agent asks for a model by — `cheap-loop`, `code-work`, `image-reader` — resolved against the bench into an ordered, priced chain of the models it can actually reach; agents never name a model, see [inference-concepts.md](inference-concepts.md) | A bench and a workbench are both tenants underneath, which can read as the same thing twice. They are not: a bench is the scope a team diff --git a/docs/connect-cards.md b/docs/connect-cards.md index a79174201..4713539b1 100644 --- a/docs/connect-cards.md +++ b/docs/connect-cards.md @@ -35,18 +35,28 @@ the agent — no settings page round-trip, no "report back when done". optional `onConnected` hook (`src/connected-hook.ts`) once the credential is durably stored. The hub wires it to `settleConnectedService` (`packages/chat`): the pending entry clears, - `chat.settings` publishes so the open card flips, and the host agent - is woken via `dispatchTurn` / `sendMail` — never by posting a timeline - row as the connecting person. + `chat.settings` publishes so the open card flips, and a + `connection.connected` event is posted — never as the connecting + person. A room waiting only under `template/pendingConnections` does + not `dispatchTurn`. A room whose own `connections/pending` named the + connector still wakes the asking agent via `dispatchTurn` / + `sendMail`. ## GitHub (Code review) GitHub for Code review is **PAT-first** today (`connect-github` block + -`packages/chat-ui` card, CL-6345): Connect opens a guided personal-access- -token paste (create a token with the `repo` scope, paste it, store -encrypted). After Connect succeeds, the same card flips in place to pick -repositories — Code review needs a repo pick before reviewers are -watching. Reviewers then introduce themselves as left-aligned messages. +`packages/chat-ui` card, CL-6345). The card is posted by +`POST /workbenches/:id/onboarding` (`packages/chat/src/routes.ts`) from +a `system@` sender when the Code review Workbench +Definition's onboarding steps ask to connect GitHub — never as a side +effect of hosting an agent, so an empty, hostless room can still run +the walkthrough. Connect opens a guided personal-access-token paste +(create a token with the `repo` scope, paste it, store encrypted). +After Connect succeeds — or when GitHub is already connected — the +same in-room card flips in place to pick repositories; there is no +`/new` already-connected dialog. Settling a credential a template room +is waiting on posts the connected notice from the system address and +never wakes an agent (`packages/chat/src/connect-pending.ts`). A GitHub App / hosted OAuth Connect as the welcome mat is CL-6343, out of scope for the shipped card — do not document OAuth-first GitHub connect