Skip to content

Commit 12c3a69

Browse files
authored
Name the frameworks in the New Bot wizard, ask visibility last, and share one harness catalogue (#799)
* Make New Bot a small ghost button * Hold the new-Bot dialog open and say what each field is for A floor of 70% of the viewport, under the 85% ceiling the popup already carried, so three of the four states land on one height instead of the dialog resizing between every question. The buttons pin to the bottom, and the pane keeps slack under it so the animated frame never shaves the last line of a step while the spring settles. "Role" beside "Title" read as a second job title when it is the standing instruction the Bot carries into every channel. It is now "What it does", and the per-channel promise moved from above all three fields onto the one field it is true of. The validation messages and the Bot's own screens follow the same name. * Move the harness catalogue into shared so both UIs read one list The twelve frameworks and the address row lived in Rust, reachable only through the Tauri command the desktop setup window calls. The web app's New Bot dialog needs the same names, summaries and marks to say "the thing you already run is one of these", and a second copy would drift — which matters more here than for most lists, because this one carries other companies' trade marks and the rules about what may be drawn for them. The desktop is not in the root bun workspace, so Rust takes the JSON by path with include_str! rather than importing the module beside it. The file is camelCase, which is what the web app reads; `Harness` stays snake_case, which is what the Tauri command has always sent HarnessPicker.tsx, so a private row struct does the renaming and the wire format is unchanged. Two fields are new and the desktop ignores both. `webCreate` is false for Mastra alone: the server reaches it as remote_mastra through the @ag-ui/mastra bridge and needs a roster id, and the web create API can send neither, so a Mastra row in that dialog would make a Bot that appears, accepts a grant, and answers nothing. `webSummary` overrides the installer's sentence on the two rows where it is false on a surface that installs nothing. * Move the vendored brand marks beside the catalogue that names them The marks sat under desktop/src, reachable only by the setup window. The web app's New Bot dialog draws the same eleven, and copying the artwork would have meant two sets of another company's trade mark to keep in step. They now live in shared/marks/, globbed from both sides, with the README that carries the rules moved with them. Both globs resolve a file to its id by stripping everything up to the marks directory rather than a fixed prefix, so neither breaks again if the path moves. The Rust guard that every named mark is vendored follows to the new location. A glob that matches nothing fails silently here: every row would simply show its name, which is also what a brand with no mark legitimately looks like. `every named mark is vendored where both apps look for it` is the assertion that tells those two apart, and it was proved to fail by removing a mark before it was trusted. * Point the composed-character test at the renamed field * Ask who can see a Bot last, and draw no step counter Visibility was the second question, which put it before the person had decided anything that makes private or public mean something. It is the one answer that does not change what they are making, so it goes last. The counter goes with the reorder rather than despite it. The wizard is three questions for a Bot that runs here and four for one at an address somebody hosts, and a counter whose total changes under the reader mid-way reads as the thing having got longer. Each step already says what it is asking in its own heading, which is what was carrying the reader. Two tests used the counter as their signal that a step had moved; they now read the heading, which says the same thing and survives the next reorder. The first version of the test forbidding the counter passed against the counter it was written to forbid — `Step {n} of {total}` renders as four text nodes and a regex matcher walks them one at a time — so it asserts against the popup's whole text instead. * Drain React's scheduler before the step test takes the document away * Offer the frameworks people already run, and ask for the address after Two radios asked a person who had just described a role to tell Built-in from Managed. Both are a kind of Bot and neither word says which one is theirs. The step now leads with the Bot this deployment runs itself and lists the frameworks somebody might already have running, by name and by their own mark, so the answer is recognised instead of deduced. The press is the answer, so there is no Continue beside them: a button that confirmed a decision already taken was the second press this path used to cost. Three frameworks show and the rest open behind a disclosure, because twelve names at once is a wall on the step somebody reaches knowing only that they want a Bot. The address moves onto a step of its own, reached only by the frameworks, so a Bot that runs here is never shown a field it has no use for. It is generic: no heading names the framework that led to it, because nothing about it differs by framework. That is also the honest limit of the list — picking Google ADK rather than CrewAI changes nothing and is stored nowhere. Mastra is absent, and shared/harnesses.json says why: the server reaches it through its own bridge and needs a roster id this create path cannot send. Tasks 4 and 5 of the plan are one commit because they could not be two. Click-to-advance leaves the step the moment a framework is pressed, so the endpoint fields could not stay inline for one commit and move in the next without shipping a state where a hosted Bot had nowhere to type an address. biome.json now skips .superpowers, the git-ignored scratch directory these plans and their mockups live in; HTML written to be thrown away was failing `bun run lint`. * Show every framework at once, and say plainly what running it here means "Show 9 more" made somebody who did not find their own in the first three press a control before they could even look. The list is open, and scrolls: scrolling is the gesture people already try on a list, and the scroller is bounded so it is the list that moves and not the step — the question above it and the Back button below it stay where they were drawn. "Run it here" was describing the deployment rather than the person's choice: "On this deployment's own Bot. Nothing to host or connect." Neither "deployment" nor "host or connect" means anything to somebody who has just written down what they want a Bot to do. It now says OpenBot runs it for you and there is nothing to set up. The unavailable case stops naming the plumbing too: it says the Bot can be switched on and that whoever set OpenBot up is the one to do it, which is true and is addressed to the right person. * Fix what the branch review found: a red build and two missing guards Four server tests still asserted "Role description must be text between 1 and 1000 characters.", which the rename changed two weeks of commits ago. CI runs the repo-wide suite, so the branch was red. This is the second half of a miss already recorded once: the app half was fixed when it was found, the server half was never looked for, because both times the rename was verified against a subset of the suite rather than all of it. `app/tests/composing-enter.test.tsx` and `create-agent-steps.test.tsx` passed only because a `mock.module` in a different file leaked process-wide. Both failed when run alone, and the plan's own step command for one of them was broken. The stub moves to app/tests/preload.ts, which exists for exactly this — its comment is about a test that "passed on its own and failed in the suite", and this was the same bug pointing the other way. A file that needs a mark drawn still overrides it. The desktop CI filter watched desktop/** only, and the catalogue and the marks it guards now live in shared/. A row with a bad `credential` would have passed lint, typecheck and the TS suite and then panicked the setup window, because `catalogue()` parses that file at runtime. The filter now watches shared/harnesses.json and shared/marks/**. Unmarked rows no longer collapse their tile. Four of the twelve have no mark, and their names began a tile's width left of the rest, which read as a ragged list rather than four brands without artwork. Reserving the column draws nothing and invents nothing; it only keeps the names in line. * Stop the scroller clipping its rows, and let the list fill the step `overflow-y-auto` computes `overflow-x` to `auto` as well, so the box clipped sideways as much as it did downward — and the inset was on the right only. The row under the pointer had its left border shaved off against that edge, which is what made the component look cut off. The padding is now on both sides, and the negative margin hands it back so the rows keep the width of the button above them. The height was a constant, chosen before seeing it in a dialog that is itself 70% of the viewport: the list stopped at 240px and left 139px of the step empty below it. In `svh` it tracks the popup it sits in — 336px of list on a laptop, and the gap down to Back is 45px instead of 139px. * Keep the step within the frame that clips it The real cause of the cut-off left edge, which the last commit guessed at and missed. The frame that animates each step's height hides its overflow on both axes. The negative margin added to keep a row's border off the clip edge made the scroller three pixels wider than that frame, and focusing the step scrolled the frame four pixels sideways to bring them into view. Every child then drew four pixels left of where it belonged — which is why the heading lost the left of its W and each row lost its left border, and why insetting on both sides did not help: the inset was the thing causing it. The scroller is flush now, and nothing inside the step is wider than the frame. The rows keep their border off the clip edge by having a focus ring drawn inside rather than outside — which they needed anyway, since they had no focus style at all and keyboard focus fell back to an outline the scroller would have clipped. The test asserts the frame's scrollWidth against its clientWidth, which is the property that actually broke. The one it replaces asserted a Tailwind class on a wrapper two levels up, and would have passed throughout.
1 parent 53a656b commit 12c3a69

35 files changed

Lines changed: 1316 additions & 344 deletions

‎.github/workflows/desktop.yml‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,16 @@ name: Desktop
33
# The shell is the one thing here that cannot be proved by running it on Linux: it is a macOS app, a
44
# Windows app and a Linux app built from one tree, and the ways they differ are exactly the ways
55
# this fails. So it tests and builds on all three, on every change to it.
6+
# `shared/harnesses.json` and `shared/marks/**` are watched here even though they are not under
7+
# desktop/: the catalogue the setup window draws moved there so the web app could read one copy, and
8+
# these jobs are the only ones that prove it still deserializes into `Harness` — `catalogue()` parses
9+
# it at runtime with an `expect`, so a bad row panics the window rather than failing a type check.
610
on:
711
pull_request:
8-
paths: ["desktop/**", "package.json", ".github/workflows/desktop.yml"]
12+
paths: ["desktop/**", "shared/harnesses.json", "shared/marks/**", "package.json", ".github/workflows/desktop.yml"]
913
push:
1014
branches: [main]
11-
paths: ["desktop/**", "package.json", ".github/workflows/desktop.yml"]
15+
paths: ["desktop/**", "shared/harnesses.json", "shared/marks/**", "package.json", ".github/workflows/desktop.yml"]
1216
workflow_call:
1317
secrets:
1418
OPENBOT_GOOGLE_MODEL_OAUTH_CLIENT_SECRET:

‎app/src/components/agents/agent-profile.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ export function AgentProfile({ agentId }: { agentId: string }) {
8989

9090
<section className="grid gap-2">
9191
<h2 className="text-xs font-medium uppercase tracking-wide text-muted-foreground">
92-
Role
92+
What it does
9393
</h2>
9494
<p className="text-sm whitespace-pre-wrap text-pretty">
9595
{profile.roleDescription}

‎app/src/components/agents/create-agent-dialog.tsx‎

Lines changed: 301 additions & 167 deletions
Large diffs are not rendered by default.
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { markFor } from "@/lib/marks";
2+
3+
/**
4+
* A framework's own mark, beside its name.
5+
*
6+
* Draws nothing when the brand has no mark. Three of the frameworks have none in any maintained
7+
* set, and `shared/marks/README.md` is explicit about why nothing is drawn to fill the gap: a
8+
* monogram we drew would read as the vendor's own, which is the thing a trade mark holder actually
9+
* objects to. The caller draws the name either way, so a row without a mark is not a lesser one.
10+
*
11+
* Decorative, always: `alt=""` and `aria-hidden`, because the name is already there in text and a
12+
* screen reader announcing "LangGraph LangGraph" is worse than one announcing it once.
13+
*/
14+
export function HarnessMark({ id, name }: { id: string | null; name: string }) {
15+
const uri = markFor(id);
16+
/*
17+
* An empty box of the same size, not nothing. Four of the twelve rows have no mark, and a
18+
* collapsed box started their names a tile's width to the left of the rest — a ragged list rather
19+
* than four brands without artwork. Reserving the column draws no mark and invents none, which is
20+
* what the README forbids; it only keeps the names in line.
21+
*/
22+
if (!uri) return <span aria-hidden="true" className="size-5 shrink-0" />;
23+
return (
24+
<img
25+
alt=""
26+
aria-hidden="true"
27+
className="size-5 shrink-0"
28+
data-mark={name}
29+
src={uri}
30+
/>
31+
);
32+
}

‎app/src/components/bot-profile/setup.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ const TEXT_FIELDS: {
6161
{ field: "title", label: "Title", icon: IconTag },
6262
{
6363
field: "roleDescription",
64-
label: "Role",
64+
// Not "Role": beside "Title" it read as a second job title, when it is the standing
65+
// instruction the Bot carries into every channel. The create dialog says the same.
66+
label: "What it does",
6567
icon: IconTextCaption,
6668
multiline: true,
6769
},

‎app/src/lib/agents/form.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ export const agentFormSchema = z.object({
1919
roleDescription: z
2020
.string()
2121
.trim()
22-
.min(1, "Role description is required.")
23-
.max(1000, "Role description must be 1000 characters or fewer."),
22+
.min(1, "A description of what it does is required.")
23+
.max(1000, "What it does must be 1000 characters or fewer."),
2424
visibility: z.enum(["public", "private"]),
2525
/**
2626
* The AG-UI endpoint this coworker runs on. Empty means the Bot in the box.

‎app/src/lib/marks.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/**
2+
* The vendored brand marks, inlined at build time as data URIs.
3+
*
4+
* The same files the desktop setup window draws, read from `shared/marks/` so there is one copy of
5+
* each vendor's artwork rather than two to keep in step. See `shared/marks/README.md` before
6+
* adding, renaming or drawing anything here: the rules in it are the reason a brand with no mark
7+
* shows its name alone instead of a monogram we invented.
8+
*
9+
* As URIs rather than SVG source on purpose: an `<img>` cannot execute anything, so drawing these
10+
* needs no `dangerouslySetInnerHTML` and what is in the file never becomes a security question.
11+
*/
12+
const files = import.meta.glob("../../../shared/marks/*.svg", {
13+
eager: true,
14+
query: "?inline",
15+
import: "default",
16+
}) as Record<string, string>;
17+
18+
const byId = new Map<string, string>(
19+
Object.entries(files).map(([path, uri]) => [
20+
path.replace(/^.*\/marks\//, "").replace(".svg", ""),
21+
uri,
22+
]),
23+
);
24+
25+
/** The mark for a brand id, or null where no maintained set has one. */
26+
export function markFor(id: string | null | undefined): string | null {
27+
return (id && byId.get(id)) ?? null;
28+
}

‎app/src/routes/_authed/_app/bots.index.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ function BotsPage() {
2222
render={(props) => (
2323
<Link {...props} search={{ new: true }} to="/bots" />
2424
)}
25-
size="lg"
25+
size="sm"
26+
variant="ghost"
2627
>
2728
<IconPlus />
2829
New Bot

‎app/tests/composing-enter.test.tsx‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,17 +172,18 @@ test("the new-coworker wizard does not move on from the Enter that confirms a co
172172
const name = await view.findByLabelText("Name");
173173
await type(name, "経費");
174174
await type(view.getByLabelText("Title"), "Finance Operations");
175-
await type(view.getByLabelText("Role"), "Review receipts.");
175+
await type(view.getByLabelText("What it does"), "Review receipts.");
176176

177177
await confirmComposedCharacter(name);
178178
await settle();
179-
expect(view.getByText("Step 1 of 3")).toBeTruthy();
179+
// The wizard draws no counter, so the step's own heading is what says where we are.
180+
expect(view.getByText("Who is this coworker?")).toBeTruthy();
180181

181182
// An ordinary Enter still means Continue.
182183
await act(async () => {
183184
fireEvent.keyDown(name, { key: "Enter", keyCode: 13 });
184185
});
185-
expect(await view.findByText("Step 2 of 3")).toBeTruthy();
186+
expect(await view.findByText("Where does it run?")).toBeTruthy();
186187
});
187188

188189
test("a boundary rule is not saved by the Enter that confirms a composed character", async () => {
Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
import {
2+
afterAll,
3+
afterEach,
4+
beforeAll,
5+
beforeEach,
6+
expect,
7+
mock,
8+
test,
9+
} from "bun:test";
10+
import { GlobalRegistrator } from "@happy-dom/global-registrator";
11+
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
12+
import {
13+
createMemoryHistory,
14+
createRootRoute,
15+
createRouter,
16+
RouterProvider,
17+
} from "@tanstack/react-router";
18+
import { act, cleanup, fireEvent, render } from "@testing-library/react";
19+
import userEvent from "@testing-library/user-event";
20+
import type { ReactNode } from "react";
21+
import { settleReactWork } from "./settle-react-work";
22+
23+
/*
24+
* `import.meta.glob` is Vite's, and `bun test` has no Vite. Stubbed the way the desktop stubs its
25+
* own marks module, keeping the shape that matters here: a brand with a vendored mark resolves to
26+
* something drawable, and one without resolves to null so its row shows the name alone.
27+
*/
28+
mock.module("@/lib/marks", () => ({
29+
markFor: (id: string | null | undefined) =>
30+
id ? `data:image/svg+xml,${id}` : null,
31+
}));
32+
33+
/*
34+
* Imported after the stub above, not beside the other imports: a static import is hoisted
35+
* and would evaluate the real marks module — and its Vite-only glob — before `mock.module`
36+
* ever ran.
37+
*/
38+
const { CreateAgentDialog } = await import(
39+
"@/components/agents/create-agent-dialog"
40+
);
41+
42+
/**
43+
* The AG-UI endpoint, on a step of its own.
44+
*
45+
* It is generic: every framework leads here and none of them changes a word of it. The step exists
46+
* only for a Bot somebody else hosts, so the Bot that runs on this deployment never sees it.
47+
*/
48+
49+
beforeAll(() => GlobalRegistrator.register());
50+
afterEach(cleanup);
51+
afterAll(async () => {
52+
await settleReactWork();
53+
GlobalRegistrator.unregister();
54+
});
55+
56+
const originalFetch = global.fetch;
57+
/** Every write the dialog sent, so "the endpoint was discarded" is an assertion. */
58+
let writes: { request: string; body: Record<string, unknown> }[] = [];
59+
60+
beforeEach(() => {
61+
writes = [];
62+
global.fetch = Object.assign(
63+
async (
64+
path: Parameters<typeof fetch>[0],
65+
init?: Parameters<typeof fetch>[1],
66+
) => {
67+
const method = init?.method ?? "GET";
68+
if (path === "/api/agents/capabilities") {
69+
return Response.json({ capabilities: { builtInAvailable: true } });
70+
}
71+
if (method === "POST" && String(path).startsWith("/api/agents")) {
72+
writes.push({
73+
request: `${method} ${path}`,
74+
body: typeof init?.body === "string" ? JSON.parse(init.body) : {},
75+
});
76+
return Response.json({ agent: { id: "expense-manager" } });
77+
}
78+
return new Response(null, { status: 404 });
79+
},
80+
{ preconnect: originalFetch.preconnect },
81+
);
82+
});
83+
84+
afterEach(() => {
85+
global.fetch = originalFetch;
86+
});
87+
88+
function draw(screen: ReactNode) {
89+
const queryClient = new QueryClient({
90+
defaultOptions: { queries: { retry: false }, mutations: { retry: false } },
91+
});
92+
const router = createRouter({
93+
history: createMemoryHistory({ initialEntries: ["/"] }),
94+
routeTree: createRootRoute({ component: () => screen }),
95+
});
96+
return render(
97+
<QueryClientProvider client={queryClient}>
98+
<RouterProvider router={router} />
99+
</QueryClientProvider>,
100+
);
101+
}
102+
103+
async function atKindStep() {
104+
const view = draw(
105+
<CreateAgentDialog onClose={() => {}} onCreated={() => {}} open />,
106+
);
107+
const name = await view.findByLabelText("Name");
108+
const user = userEvent.setup({ document: name.ownerDocument });
109+
await user.type(name, "Expense Manager");
110+
await user.type(view.getByLabelText("Title"), "Finance Operations");
111+
await user.type(view.getByLabelText("What it does"), "Review receipts.");
112+
await user.click(view.getByText("Continue"));
113+
await view.findByText("Where does it run?");
114+
return { view, user };
115+
}
116+
117+
test("a framework leads to one generic endpoint step", async () => {
118+
const { view, user } = await atKindStep();
119+
await user.click(view.getByRole("button", { name: /CrewAI/ }));
120+
const field = await view.findByLabelText("Agent endpoint");
121+
expect(field).toBeTruthy();
122+
/*
123+
* Generic: the framework that led here is not named on it. Scoped to this step's own pane
124+
* rather than the popup, because the pane being slid out is still mounted mid-animation and
125+
* carries the name of the button that was just pressed.
126+
*/
127+
const pane = view
128+
.getByText("Where can it be reached?")
129+
.closest('[data-slot="questionnaire-item"]');
130+
expect(pane?.textContent).not.toContain("CrewAI");
131+
});
132+
133+
test("the Bot that runs here skips the endpoint step entirely", async () => {
134+
const { view, user } = await atKindStep();
135+
await user.click(view.getByRole("button", { name: /Run it here/ }));
136+
expect(await view.findByText("Who can see it?")).toBeTruthy();
137+
expect(view.queryByLabelText("Agent endpoint")).toBeNull();
138+
});
139+
140+
test("going back keeps the framework that was chosen", async () => {
141+
const { view, user } = await atKindStep();
142+
await user.click(view.getByRole("button", { name: /CrewAI/ }));
143+
await view.findByLabelText("Agent endpoint");
144+
await user.click(view.getByText("Back"));
145+
const chosen = await view.findByRole("button", { name: /CrewAI/ });
146+
expect(chosen.getAttribute("aria-pressed")).toBe("true");
147+
});
148+
149+
/*
150+
* Review Focus: an endpoint typed on the way past must not ride along into a Bot that runs here.
151+
* The server stores whatever it is sent, and a built-in Bot carrying somebody's stale URL would be
152+
* dialled at it.
153+
*/
154+
test("choosing Run it here discards an endpoint typed earlier", async () => {
155+
const { view, user } = await atKindStep();
156+
await user.click(view.getByRole("button", { name: /CrewAI/ }));
157+
await user.type(
158+
await view.findByLabelText("Agent endpoint"),
159+
"https://example.com/ag-ui",
160+
);
161+
await user.click(view.getByText("Back"));
162+
await user.click(view.getByRole("button", { name: /Run it here/ }));
163+
await view.findByText("Who can see it?");
164+
await user.click(view.getByText("Create coworker"));
165+
await act(async () => {
166+
await new Promise((resolve) => setTimeout(resolve, 50));
167+
});
168+
expect(writes).toHaveLength(1);
169+
expect(writes[0]?.body.endpoint).toBe("");
170+
});
171+
172+
test("Enter in the endpoint field advances the step", async () => {
173+
const { view, user } = await atKindStep();
174+
await user.click(view.getByRole("button", { name: /CrewAI/ }));
175+
const field = await view.findByLabelText("Agent endpoint");
176+
await user.type(field, "https://example.com/ag-ui");
177+
await act(async () => {
178+
fireEvent.keyDown(field, { key: "Enter", keyCode: 13 });
179+
});
180+
expect(await view.findByText("Who can see it?")).toBeTruthy();
181+
});

0 commit comments

Comments
 (0)