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/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 3b56fdc8d..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,65 +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, - ...(manifest !== undefined ? { templatePromise: manifest.promise } : {}), - ...(requiresGithub && !githubAlreadyConnected && manifest !== undefined - ? { connectGithubRequiredFor: manifest.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) => ({ @@ -232,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), }); @@ -249,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 9370b901a..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; }); @@ -563,8 +558,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, @@ -594,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: [ @@ -652,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", @@ -666,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; }); @@ -715,52 +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 createWorkbenchCall = calls.find( - (call) => - call.path.endsWith("/chat/workbenches") && call.init?.method === "POST", + expect(document.body.textContent).not.toContain( + "Choose repos this workbench can work on", ); expect( - JSON.parse(String(createWorkbenchCall?.init?.body)), - ).not.toHaveProperty("connectGithubRequiredFor"); - - const startReviewingCall = calls.find((call) => - call.path.endsWith("/workbenches/chan_new/github/start-reviewing"), - ); - expect(startReviewingCall).not.toBeUndefined(); + Array.from(document.querySelectorAll("button")).some((button) => + button.textContent?.startsWith("Start reviewing"), + ), + ).toBe(false); }); }); 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 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..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,19 +693,16 @@ 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", - templatePromise: manifest.promise, + kind: "workbench", + name: definition.title, }; - if (manifest.requiredConnections.includes("github")) { - createBody["connectGithubRequiredFor"] = manifest.title; - } const createRes = await api( hub.baseUrl, "POST", @@ -720,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, @@ -791,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"/); +});